Skip to content
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

Update rust version to 1.80 #1805

Merged
merged 9 commits into from
Aug 5, 2024
Merged

Conversation

orpuente-MS
Copy link
Contributor

@orpuente-MS orpuente-MS commented Jul 30, 2024

This PR updates the versions in our build and release pipelines to use Rust 1.80.

When fixing the new clippy lints to satisfy 1.80, we get some errors in the build pipeline.

All of the errors are related to the fact that rust 1.80 implements Iterator for &Box<[...]>. So, what before had to be written as for elt in container.iter() now can be written as for elt in container which is more concise, and clippy enforces this new style.

for elt in &**container happens to be equivalent and it works both in rust 1.78 and rust 1.80, and clippy doesn't complain about it in either.

This PR changes all the instances of for elt in container.iter() where container is a &Box<[...]> to for elt in &**container so we can update the rust version to 1.80. There will be a subsequent PR that changes all the instances of for elt in &**container to for elt in container.

Copy link

Benchmark for e9e2219

Click to view benchmark
Test Base PR %
Array append evaluation 339.0±3.35µs 338.1±2.98µs -0.27%
Array literal evaluation 177.2±7.88µs 193.7±1.47µs +9.31%
Array update evaluation 415.6±1.68µs 412.4±4.59µs -0.77%
Core + Standard library compilation 26.2±1.16ms 25.8±1.28ms -1.53%
Deutsch-Jozsa evaluation 5.0±0.07ms 5.0±0.05ms 0.00%
Large file parity evaluation 34.1±0.18ms 34.1±0.11ms 0.00%
Large input file compilation 15.0±0.61ms 14.9±0.65ms -0.67%
Large input file compilation (interpreter) 58.4±2.28ms 58.4±2.22ms 0.00%
Large nested iteration 33.1±0.21ms 32.5±0.44ms -1.81%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1628.0±140.79µs 1624.4±141.84µs -0.22%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.3±0.12ms 8.3±0.14ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1495.1±212.82µs 1480.4±137.73µs -0.98%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 28.8±0.40ms 28.8±0.46ms 0.00%
Teleport evaluation 92.0±3.49µs 92.2±3.60µs +0.22%

Copy link

Benchmark for 5bee0a8

Click to view benchmark
Test Base PR %
Array append evaluation 342.4±4.58µs 336.1±2.23µs -1.84%
Array literal evaluation 174.6±1.49µs 192.3±1.36µs +10.14%
Array update evaluation 420.7±11.09µs 412.1±3.40µs -2.04%
Core + Standard library compilation 21.9±0.60ms 21.6±0.55ms -1.37%
Deutsch-Jozsa evaluation 4.9±0.06ms 4.9±0.04ms 0.00%
Large file parity evaluation 34.0±0.34ms 34.0±0.47ms 0.00%
Large input file compilation 12.9±0.32ms 12.5±0.15ms -3.10%
Large input file compilation (interpreter) 49.8±1.53ms 48.5±1.18ms -2.61%
Large nested iteration 33.2±0.37ms 32.6±0.70ms -1.81%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1584.5±75.98µs 1568.1±60.37µs -1.04%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.11ms 7.7±0.06ms -1.28%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1449.9±100.77µs 1422.0±45.85µs -1.92%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.9±0.52ms 27.5±0.50ms -1.43%
Teleport evaluation 91.9±3.98µs 91.6±3.19µs -0.33%

Copy link

Benchmark for a8e0c6e

Click to view benchmark
Test Base PR %
Array append evaluation 339.7±3.44µs 341.2±4.20µs +0.44%
Array literal evaluation 175.1±1.09µs 174.9±0.99µs -0.11%
Array update evaluation 413.5±1.15µs 415.9±2.72µs +0.58%
Core + Standard library compilation 21.5±0.23ms 21.5±0.26ms 0.00%
Deutsch-Jozsa evaluation 4.9±0.05ms 4.9±0.06ms 0.00%
Large file parity evaluation 34.0±0.07ms 33.9±0.11ms -0.29%
Large input file compilation 12.9±0.23ms 12.9±0.15ms 0.00%
Large input file compilation (interpreter) 50.7±1.07ms 50.1±1.10ms -1.18%
Large nested iteration 33.0±0.17ms 33.1±0.20ms +0.30%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1559.4±31.34µs 1561.1±41.50µs +0.11%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.12ms 7.7±0.07ms -1.28%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1424.1±56.72µs 1422.3±32.95µs -0.13%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.6±0.46ms 27.6±0.14ms 0.00%
Teleport evaluation 91.9±3.50µs 91.1±3.56µs -0.87%

@ScottCarda-MS
Copy link
Contributor

Based on the description of the PR, why not just updated all instances of for elt in container.iter() where container is a &Box<[...]> to for elt in container instead of for elt in &**container?

@orpuente-MS
Copy link
Contributor Author

orpuente-MS commented Jul 31, 2024

Based on the description of the PR, why not just updated all instances of for elt in container.iter() where container is a &Box<[...]> to for elt in container instead of for elt in &**container?

Because our current pipeline is built using rust 1.78 and impl<T> Iterator for Box<[T]> was introduced in rust 1.80. So, for elt in container issues a compile error in the current build pipeline.

@orpuente-MS orpuente-MS added this pull request to the merge queue Aug 5, 2024
Merged via the queue into main with commit a5a7a90 Aug 5, 2024
19 checks passed
@orpuente-MS orpuente-MS deleted the oscarpuente/update-rust-version-1.80 branch August 5, 2024 20:41
github-merge-queue bot pushed a commit that referenced this pull request Aug 5, 2024
PR #1805 changed all instances of `for elt in container.iter()` where
container was a `&Box<[...]>` to `for elt in &**container`, which is
equivalent, to be able to update our build and release pipelines to rust
1.80, you can see the full explanation there.

This is a subsequent PR that changes:
 - All instances of `for elt in &**container` to `for elt in container`.
 - All instances of `for elt in &*container` to `for elt in &container`.

These changes are due to rust 1.80 implementing `Iterator` for
`Box<[...]>` and clippy enforcing a new style of iteration.
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