Fix attribute order implementation#154781
Conversation
rustbot
added
A-attributes
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Fix attribute order implementation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
Footnotes
|
craterbot
added
S-waiting-on-review
|
All crater regressions & fixed look spurious to me. |
|
Some changes occurred in compiler/rustc_attr_parsing |
|
@bors r+ |
rust-bors
Bot
added
S-waiting-on-bors
…uwer Rollup of 5 pull requests Successful merges: - #154781 (Fix attribute order implementation) - #155242 (resolve: Introduce `(Local,Extern)Module` newtypes for local and external modules respectively) - #149614 (Use `MaybeDangling` in `std`) - #153178 (Add `TryFromIntError::kind` method and `IntErrorKind::NotAPowerOfTwo` variant) - #155049 (Documenting the case of `Weak::upgrade` returning `None` when the value behind the reference is missing) Failed merges: - #155308 (Make `OnDuplicate::Error` the default for attributes)
…uwer Rollup of 5 pull requests Successful merges: - #154781 (Fix attribute order implementation) - #155242 (resolve: Introduce `(Local,Extern)Module` newtypes for local and external modules respectively) - #149614 (Use `MaybeDangling` in `std`) - #153178 (Add `TryFromIntError::kind` method and `IntErrorKind::NotAPowerOfTwo` variant) - #155049 (Documenting the case of `Weak::upgrade` returning `None` when the value behind the reference is missing) Failed merges: - #155308 (Make `OnDuplicate::Error` the default for attributes)
Rollup merge of #154781 - JonathanBrouwer:fix-attr-order, r=jdonszelmann Fix attribute order implementation The implementation in #153041 contained a mistake :c I swapped the place where the error message was on, but did not change any code for which attribute was also selected, which explains the empty crater results. **Interestingly, the original code is broken too, before #153041 it always took the last attribute, regardless of what the `AttributeOrder` actually was...** Let's first see crater results before making a decision TODO for this PR: Make better tests for this
|
@rust-timer build 7131292 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (7131292): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (secondary 2.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -3.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 490.91s -> 491.403s (0.10%) |
5 participants
The implementation in #153041 contained a mistake :c
I swapped the place where the error message was on, but did not change any code for which attribute was also selected, which explains the empty crater results.
Interestingly, the original code is broken too, before #153041 it always took the last attribute, regardless of what the
AttributeOrderactually was...Let's first see crater results before making a decision