Hey @LeSeulArtichaut, I'd like to help move that PR forward. The error you're currently stuck on is very weird. It'll be a while before I can poke at it myself, in the mean time can you tell me:
rustcinvocation are you using that gives you the expected error?
Oh, hello @Hanna Kruppe, thanks for reaching out. I'm currently trying to debug this by manually adding the flags
rusttest uses to try to find something interesting
rustc invocation I'm using is this one:
rustc +stage1 src/test/ui/rfcs/rfc-2396-target_feature-11/trait-impl.rs
stage1 is a toolchain I set up as the rustc-dev-guide recommands
./x.py invocation I'm using is:
./x.py test -i --stage 1 src/test/ui/rfcs/rfc-2396-target_feature-11/trait-impl.rs
Aha, I think I narrowed it. Using
--emit metadata makes the test compile!
Ohhh, the check is in
codegen_fn_attrs which is not run when not generating code!
It might make more sense in
Alright, I'll come back in 1 hour once my computer has painfully compiled
More seriously though, thanks @Hanna Kruppe, I'd never have found out this detail myself
How did it go, @LeSeulArtichaut? Anything else I can do to help?
Something still feels wrong to me. Take the error when
#[target_feature] is applied to a safe function as it is today. It is emitted when using
--emit metadata, even though it is located in
Oh, hm, you're right. I missed this call which forces
codegen_fn_attrs to be computed in any case https://github.com/rust-lang/rust/blob/2917d993023dec5111147a1552ec78b206a5a37e/src/librustc_passes/check_attr.rs#L81
Now that I'm looking more carefully, there are also a few other places in type checking etc. that query the codegen attrs
Sanity check: does this
debug! never execute with
--emit metadata but prints something without
(Poke @Hanna Kruppe)
So I just realized that the aforementioned call to
codegen_fn_attrs only applies to free functions, not methods. I'm seeing a few other places that I would expect to call it anyway (1 2), but IMO it's worth trying if adding a
|| target == Target::Method in check_attr.rs helps
This seems to work. I ran
./x.py test on the whole test suite and got no other error than minor UI breakages related to the feature
I've pushed my local branch to update the PR
Hi @Hanna Kruppe, I've seen @NeoRaider's PR (#71205) to resolve the issue with attributes on closures, so I wondered if I could rebase my PR on his. What do you think?
I don't mind. It potentially means your PR will wait a little longer (that other PR doesn't seem to move very quickly either), but I guess it also depends on when you rebase and when I get the time to look at it again for the final r+.
If I’m the only one concerned, I don’t mind that the PR will wait longer. The reason why I’d like to rebase is because of the UI failures that I had encountered, and that will probably be fixed by #71205.
I think I’ll try to rebase locally and see where that leads me
Makes sense :+1:
Maybe I can help push #71205 in return? :D
Well, I think it is waiting for review, or someone who understands why my change fixes more things than expected? It's been lying there for a week, so I assume it will be triaged soon anyways
Hopefully this now passes the CI tests :fingers_crossed: