Rust: asm!: `options` should work multiple times, to simplify macros

[joshtriplett]

To simplify the implementation of macros wrapping asm!, I think options should work if specified multiple times, with the semantic that no conflicting options may be provided. (We could also do "last option wins", but I think I'd slightly prefer "don't specify conflicting options" instead.)

This would, for instance, make it much easier to implement an asm_att! macro that passes all its arguments to asm! and adds options(att_syntax), without having to scan all the tokens it receives, look for an existing options, and append to it if found.

Sample code that I think should work (printing 9):

#![feature(asm)]

fn main() {
    let mut i: u32 = 5;
    unsafe {
        asm!(
            "add $4, %rax",
            inout("rax") i,
            options(att_syntax),
            options(nostack),
        );
    }
    println!("{}", i);
}

[Amanieu]

That seems like an extremely niche use case that would only really enable an asm_att! macro and nothing else. I don't expect such a macro to actually be used in practice since adding options(att_syntax) is already very easy.

[joshtriplett]

A large codebase that uses primarily AT&T syntax will want to spell it as briefly as possible; I expect to see such a macro in common use.

If you believe this would complicate the parser unnecessarily, then I would be open to a crate-based solution that shows how to implement the macro taking existing options into account.

[cesarb]

with the semantic that no conflicting options may be provided. (We could also do "last option wins", but I think I'd slightly prefer "don't specify conflicting options" instead.)

I think a simpler semantic would be "multiple options is identical to a single options with the options concatenated". That is, options(A, B), options(C, D) would have the exact same semantics as options(A, B, C, D).

This would be good not only for macros, but also for readability (some people might prefer multiple options, one on each line, instead of being forced to use a single options, when the list of options starts to get too long).

[joshtriplett]

@cesarb That sounds reasonable to me.

[Amanieu]

This is actually a very trivial change to the parsers, so it should be easy to add.

[camelid]

@rustbot claim

[camelid]

Oops, didn't realize it was claimed! Sorry

@rustbot release-assignment

I assume you were planning to do this, @Amanieu?

[Amanieu]

No no, go ahead, you can have it.

[camelid]

Thanks! I’ll give it a try :)

@rustbot claim

[camelid]

How should I handle diagnostics? There are multiple errors that show where the error in the options occurred, e.g.:

ecx.struct_span_err(span, "the `nomem` and `readonly` options are mutually exclusive")
    .emit();

The way I'm doing the multiple options is to just let it bitwise-OR the different flags into the AsmArgs.options field, and instead of having options_span: Option<Span>, have options_spans: Option<Vec<Span>>. The problem is that then you can't tell where the options came from. Should I change options to be a Vec<ast::InlineAsmOptions>?

Advice?

[camelid]

Also, perhaps there should be a simple diagnostic for checking for duplicate options?

[camelid]

Alternatively, the errors could just be reported on all the options spans.

[Amanieu]

Error reporting can take a Vec<Span> to report multiple error sources at once.

[Amanieu]

You can add a diagnostic for setting the same option multiple times by just checking if it has already been ORed in.

[camelid]

You can add a diagnostic for setting the same option multiple times by just checking if it has already been ORed in.

Yeah, that's what I was thinking

[camelid]

Error reporting can take a Vec<Span> to report multiple error sources at once.

Is it okay to .clone() the Vec<Span>?

[Amanieu]

Sure.

[nagisa]

Last option overrides is a better design if the goal is to enable macros. That way invocations of the same asm! with att-syntax-by-default can still opt into intel syntax by appending what otherwise would be a conflicting option.

(EDIT: OTOH I think nomem vs readonly strongly informs the design towards no conflicting options)