Lower attributes for functions without bodies - #162761
JonathanBrouwer wants to merge 3 commits into
Conversation
|
|
This comment has been minimized.
This comment has been minimized.
29274ef to
5bf5ae2
Compare
|
|
||
| warning: the `must_use` attribute cannot be used on function params | ||
| --> $DIR/param-attrs-builtin-attrs.rs:40:7 | ||
| --> $DIR/param-attrs-builtin-attrs.rs:29:7 |
There was a problem hiding this comment.
the check for these warnings should go, since it causes double emissions
There was a problem hiding this comment.
Let's fix this in a follow-up PR, it will be quite a big diff to remove this error and replace it by target checking consistently
|
@rust-lang/lang Is this a PR that you would want to take a look at? fn help(x: fn(#[rustc_splat] usize)) {} |
|
@rustbot ready |
5bf5ae2 to
e6552cd
Compare
|
r=me then after some acknowledgement from t-lang. I think it falls under the same rule as earlier breaking changes in attrs (i.e. notify t-lang but do the change) but let them acknowledge that. Notably, this attribute is rather new, it's unlikely anyone depends on this. |
|
Just in case |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Lower attributes for functions without bodies
| //~| ERROR allow, cfg, cfg_attr, deny, expect, forbid, and warn are the only allowed built-in attributes | ||
| } | ||
|
|
||
| trait Test { |
There was a problem hiding this comment.
I don't know if it's worth duplicating the trait, type, and extern tests in the splat feature gate test, up to you.
There was a problem hiding this comment.
I think it's not worth duplicating the test
This comment has been minimized.
This comment has been minimized.
e6552cd to
cad361c
Compare
|
Don't think a stable backport is worth it. We can consider a beta backport, I opened a thread for it |
|
Finished benchmarking commit (ed70b09): comparison URL. Overall result: ❌✅ regressions and improvements - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -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)This perf run didn't have relevant results for this metric. CyclesResults (primary 1.5%, secondary -8.5%)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.332s -> 494.947s (-0.08%) |
View all comments
Fixes #162639
This is a temporary solution to get the bug fixed.
I'm going to see if we can move the function
hir::Params fromhir::Bodytohir::FnSig, this would cleanly fix this bug and would undo most of this PR. That is a large project which might take me some time to complete, so I'd prefer to get the bug fixed first with this workaround.Maybe r? @jdonszelmann since you have context?
cc @mejrs cause you're probably interested