Repository navigation
port lint attributes - #162811
port lint attributes#162811
Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs cc @jdonszelmann, @JonathanBrouwer These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_attr_ir cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer
cc @rust-lang/clippy |
| LL | let y: u32 = (x?).try_into().unwrap(); | ||
| | + +++++++++++++++++++++ |
There was a problem hiding this comment.
Uh yeah I don't know what's happening here
There was a problem hiding this comment.
I think this is minor enough that I'll put it on my todo list as something that we should fix after merging this PR
|
I'll take a look at this on Friday! Thanks for doing this! |
|
Thanks for picking this up ❤️ |
5f087e2 to
c90513f
Compare
|
cc @cjgillot @petrochenkov in case you'd like to take a look (as reviewers of the original pr #155691) |
I'll go ahead and cherrypick some changes from this PR then, some things can be split off. |
|
Blocked on #162813 |
This comment has been minimized.
This comment has been minimized.
c90513f to
5c86bdf
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (837b948): comparison URL. Overall result: ❌✅ regressions and improvements - BENCHMARK(S) FAILEDBenchmarking 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. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never rustc-perf ❗ ❗ ❗ ❗ ❗
❗ ❗ ❗ ❗ ❗ Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -7.1%, secondary -1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -7.6%, secondary 2.7%)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: 495.925s -> 496.275s (0.07%) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Some thoughts on some of them:
with
If macro lints ever happen (in the form of #135432 or smth else) we likely need a tcx to resolve these lints, which cannot be done in attribute parsing
a "downcast_slice" method could also exist in std or e.g. rustc_data_structures.
I think a "just try to parse this without actually erroring" might work (we kinda do this for diagnostic attributes), but it's a bit brittle for my liking, I'm also not sure I like Another option is to force pre-expansion lints to be crate level only (like #163714) but this would also be a lang (affects new_keyword lint) and clippy matter.
|
This comment has been minimized.
This comment has been minimized.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
There was a problem hiding this comment.
I'm looking at this, which is the rangediff since I last reviewed the PR
https://triagebot.infra.rust-lang.org/gh-range-diff/rust-lang/rust/a639ea0890c0977b1c45596fd6b5f9a70a1d67da..89b2c2ca86143266325fc3ddc3ae46d6b8077850/91e426a0518d5bb3672330c5238ccda6fd26fb54..ea4a0bd0643d32eddc2f0007ae38c09ec38aeed9
Why was the change in compiler/rustc_mir_build/src/thir/cx/expr.rs removed?
I liked having the lint attr check there
There was a problem hiding this comment.
(actually r=JonathanBrouwer,petrochenkov)
Thanks so much for the hard work in getting this attempt merged @mejrs
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@bors rollup=never note=more likely than average to cause regressions, perf |
|
@bors p=6 |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 1e16699 (parent) -> 822182d (this PR) Test differencesShow 40 test diffs40 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 822182db4c4a1827256881d8c7aa685f0bcf2b11 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (822182d): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 4.5%, secondary 5.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -6.0%)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: 487.497s -> 485.154s (-0.48%) |
|
@rustbot label: +perf-regression-triaged This matches expectations. |

View all comments
Ports the lint attributes. I've kept this as small as possible, but there is a decent amount of clean up/refactoring that can be done afterwards,
r? @JonathanBrouwer @jdonszelmann
cc @Bryntet