port lint attributes - #162811
port lint attributes#162811mejrs wants to merge 4 commits into
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
|
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. |
| #[allow()] | ||
| { } | ||
| } | ||
| fn expr_block() { { } unsafe { } 'a: { } { } { } } |
There was a problem hiding this comment.
We drop these empty lint attributes, this is nice because when we query for the attribute we know it's non-empty and we can do things like lints.first().unwrap()
|
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.
|
Finished benchmarking commit (6091374): comparison URL. Overall result: ❌ regressions - please read:Benchmarking 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 -2.4%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.4%, secondary 3.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.1%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 497.193s -> 499.023s (0.37%) |
|
@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 (4b49dca): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking 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 -1.1%, secondary -2.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -0.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: 502.551s -> 499.436s (-0.62%) |
|
@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.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9ae8a4a): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking 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 -2.6%, secondary -0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.8%, secondary 0.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: 502.551s -> 496.987s (-1.11%) |
| DUMMY_SP, | ||
| CRATE_NODE_ID, | ||
| Some(self.features), | ||
| ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed }, |
There was a problem hiding this comment.
if you make this ShouldEmit::Nothing then the majority of all the duplicate lints go away. Issue is that this leads to delay_bug from pre-ast-expansion parsing which leads to ICE
there is a way to do this a bit hackily (basically in lintcheck parsing, check if ShouldEmit::Nothing and then never return an error)
there is also the possibility of changing how pre-ast-expansion emits lints. I've been considering making a PR where we change so that pre-ast-expansion doesn't parse lint attrs but instead just saves diagnostic + span and then that gets sent to the lint store that we instantiate later.
There was a problem hiding this comment.
there is a way to do this a bit hackily (basically in lintcheck parsing, check if
ShouldEmit::Nothingand then never return an error)
I don't really want to do this (especially not in this PR), it sounds rather brittle.
In any case I would prefer to just not parse these attributes at all, perhaps we can force all all pre-expansion lints to be crate level only which would sidestep this problem.
39d49a1 to
ca33292
Compare
This comment has been minimized.
This comment has been minimized.
ca33292 to
e7da601
Compare
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