Skip to content

Update to Ceno's version of the toolchain #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Nov 18, 2024

Conversation

matthiasgoergens
Copy link
Contributor

@matthiasgoergens matthiasgoergens commented Oct 18, 2024

We are getting a lot of flak from dependabot about security problems in dependencies.

So let's update our dependencies to fix them.

Also rename to config.toml to make the newer cargo happy.

@matthiasgoergens
Copy link
Contributor Author

The digest dependency dropped SIMD related features. I assume they are turned on by default now.

Also rename to `config.toml` to make the newer cargo happy
@matthiasgoergens matthiasgoergens force-pushed the matthias/update-toolchain branch from e7eb575 to dda1bbc Compare October 18, 2024 04:49
@@ -24,17 +24,6 @@ pub enum Token {
// Identifiers
#[regex(br"#t|#a|#l|#m|[^()0-9#; \t\n\f][^(); \t\n\f#]*")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one clashed with the 'Literals' section above. I suggest perhaps fixing this regex? For now, I just bumped the priority on the Literals.

let mut init_vir_mems_list = init_vir_mems_list.into_iter().map(|v| v.assignment).collect_vec();
let mut addr_phy_mems_list = addr_phy_mems_list.into_iter().map(|v| v.assignment).collect_vec();
let mut addr_vir_mems_list = addr_vir_mems_list.into_iter().map(|v| v.assignment).collect_vec();
let mut addr_ts_bits_list = addr_ts_bits_list.into_iter().map(|v| v.assignment).collect_vec();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any sayings about using collect::<Vec<_>> instead of collect_vec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, they are basically the same. It's just that collect comes from the standard library, and collect_vec comes from itertools.

Let me try to remember why this one changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I figured it out: collect_vec wasn't available by default anymore. So we would have needed to import it. But this was cleaner.

@matthiasgoergens matthiasgoergens force-pushed the matthias/update-toolchain branch from faeb390 to b9db585 Compare November 18, 2024 07:14
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually just a rename to spartan_parallel/.cargo/config.toml

ff/Cargo.toml Outdated
ff_derive = { version = "0.12.1", path = "ff_derive", optional = true }
rand_core = { version = "0.6", default-features = false }
subtle = { version = "2.2.1", default-features = false, features = ["i128"] }
ff_derive = { version = "0", path = "ff_derive", optional = true }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems not a good idea. v0.x and v0.y don't need to be compatible in sem ver

ff_derive = { version = "0",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They actually need to be compatible in semantic versioning. But you are right that in practice they often aren't.

Fixed.

@matthiasgoergens matthiasgoergens merged commit adc0460 into main Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants