Allow unused arguments in asm! #73056
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I don't understand the motivation. |
petrochenkov
added
S-waiting-on-author
|
The only use case that I know of is #![feature(asm)]
pub fn black_box<T>(mut dummy: T) -> T {
unsafe {
asm!("", in(reg) &mut dummy, options(nostack, preserves_flags));
dummy
}
} |
|
@Amanieu In other words, what it the difference between the example above and pub fn black_box<T>(dummy: T) -> T {
unsafe {
asm!("", options(nostack, preserves_flags));
dummy
}
}? |
|
(actually there's a slight mistake in my code, it should be The point of |
|
Looks like I misinterpreted |
|
I'm happy to update the code if you can suggest a better name! |
|
Also, an alternative to this PR would be to turn the unused argument error into a warning that can be suppressed with |
petrochenkov
added
S-waiting-on-review
|
By the way, does inline assembly support comments? If it does, then will make it look used for rustc. UPD: Yep, it works at least for x86 asm. |
|
It does but different architectures use different symbols for comments:
This makes it a bit of a mess to write a portable version of |
|
@Amanieu So, I'd personally prefer not extend the syntax util a case for which |
petrochenkov
added
S-waiting-on-author
|
I've reverted the syntax changes. I went with the other approach to simply change unused arguments from a hard error to a warn-by-default lint. I feel that we should do this even though you can work around it with comments since the comment hack isn't very discoverable. |
You cannot guarantee that a future assembly syntax for a future ISA will not use /* for a non-comment purpose. That is, if you depend on comments to fool the "unused arguments" error, you need a comment syntax that is guaranteed to be the same for every current or future ISA. That said, There are at least two main cases that I can think of: register value ( So I think that the simplest and most flexible approach is to just allow arguments to be missing from the template string, but still be treated as used by the compiler, either through a lint which can be allowed, or through a special named argument or some other syntax trickery. There is a lot of precedent for that (not only the Rust |
|
Hmm, this raises some compatibility questions regarding C-style comments in Looks like It means that
This is a more general issue, not specific to unused arguments. |
|
Fixed the lint name and squashed. |
This comment has been minimized.
This comment has been minimized.
|
Lint makes sense to me, but why warn-by-default? Users who end up needing to have unused arguments in their code will want to (Furthermore decreasing the lint level is something that cannot be undone later – but we can always downgrade the default severity later if we think we need to still) |
|
Most of our deny-by-default lints are for future-incompat issues or things that will panic at runtime (e.g. overflow). It felt more consistent to make it warn by default like all the other unused X lints. |
petrochenkov
added
S-waiting-on-review
petrochenkov
added
S-waiting-on-author
petrochenkov
added
S-waiting-on-review
|
I somewhat got it working by moving the unused argument check to the AST lowering pass where the node ID of the // Works
#[allow(unused_asm_arguments)]
{
asm!("", in(reg) 0, in(reg) 1);
}
// Doesn't work
#[allow(unused_asm_arguments)]
asm!("", in(reg) 0, in(reg) 1); |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
It's good that this can be reported after expansion, when all |
|
AST lowering is not a good place for that though, it should be possible to add the |
That's just how lints and macros work in general - macro calls don't have node ids to which lint levels can be attached (also see #63221). Given the lint inconveniences and #72016 (comment) perhaps it's time to reconsider |
|
However in this case the macro expands to an |
|
In fact if I wrap the #[allow(unused_asm_arguments)]
(asm!("", in(reg) 0, in(reg) 1));I think it should be possible to make the |
|
That what #63221 is about - the current language behavior is that inert attributes on (It may be possible to change it, but it's a big and potentially breaking change.) EDIT: it may also be possible to change built-in |
|
Closing in favor of #73230. |
This PR changes the "unused arguments" error for
asm!to a warn-by-default lint.Fixes #72965