Rust: [nightly regression] rustc incorrectly parses attributes as macro invocations in nightly-2018-08-18

[jchlapinski]

In the newest nightly all attributes with names identical to existing macros are parsed as macro invocation (note the lack of ! at the front of dummy in the example below).
Previous version nightly-2018-08-17 works as expected.
This breaks compilation of some of our custom derives, using same name for attributes and macros, which was previously allowed.

I tried this code:

macro_rules! dummy {
    ($x:expr) => {
        println!("dummy: {}", $x)
    }
}

#[dummy("text")] // this should not be regarded as a `dummy` macro invocation, should it?
struct SampleStruct {

}

fn main() {
    dummy!("text");
}

I expected this to complain about unknown custom attribute dummy

Instead, I got this error:

error: macro `dummy` may not be used in attributes
 --> src/main.rs:7:1
  |
7 | #[dummy("text")]
  | ^^^^^^^^^^^^^^^^

Meta

rustc --version --verbose:

rustc 1.30.0-nightly (33b923fd4 2018-08-18)
binary: rustc
commit-hash: 33b923fd44c5c5925e635815fce68bdf1f98740f
commit-date: 2018-08-18
host: x86_64-unknown-linux-gnu
release: 1.30.0-nightly
LLVM version: 7.0

[petrochenkov]

Currently this is expected behavior - fn-like macros, attribute macros and derive macros share single namespace and can shadow each other (similarly to types being able to shadow traits etc).
See https://github.com/rust-lang/rust/issues/53205#issuecomment-411599806 for some motivation.

This change shouldn't affect stable Rust since attribute macros were stabilized only few days ago.

[jchlapinski]

... fn-like macros, attribute macros and derive macros share single namespace and can shadow each other (similarly to types being able to shadow traits etc).

Ok, but how is this #[dummy("aaa")] a "attribute macro" instead of just an fn-like attribute? My understanding is that in order for it to be an attribute macro it should read #[dummy!("aaa")], no?

[nikomatsakis]

@jchlapinski can you say a bit more about the actual code that is breaking for you?

cc @rust-lang/lang

[nikomatsakis]

Flagging this for @rust-lang/lang discussion -- do we want all macro_rules and other macros to be usable as macros in attribute position? It's certainly something we've talked about, but I thought it'd be good to "check in" on this.

[nikomatsakis]

In particular, if this is regressing code, it may not be something we can't really do. (Especially stable code -- it's still not clear to me just what the regressed code looks like.)

[Centril]

The error message seems a bit confusing as I think it leads the user to believe that dummy was called inside the attribute (so it would have been expanded to #[println!("dummy: {}", "text")].

If you actually wanted that behavior then the invocation should have been #[dummy!("text")] which may be useful for some sort of attribute-aliasing. However; I think such a step would require some more thinking into the design and possible alternatives, etc.

However, I think @petrochenkov's reasoning is right; this is not a stable-to-nightly regression because attribute macros were not previously stable. If you use an attribute name with the same name as a macro_rules macro, then the compiler is not to blame here imo.

See https://github.com/rust-lang/rust/issues/53583#issuecomment-415450642 for why it is a stable-to-nightly-regression. @petrochenkov's solution seems like a good one provided that attribute proc macros can be renamed to deal with conflicts.

[jchlapinski]

@nikomatsakis basically we have custom derive for our diagnostic type trait Diag, which is using an fn-like attribute named diag and also a macro (defined with macro_rules) with that same name. Macro is for instantiation of out diagnostic type, where we do some backtrace generation, logging, etc.

Now we can of course change either name, however I still do not understand why using same name for attribute AND a macro would clash? By that same logic I can have a field and a method with the same name, since field access and method calls are easily discerned in parsing. Is there a different way to expand a macro, without suffixing macro name with a !?

[jchlapinski]

@Centril I am not blaming, I am just reasoning on why those things are sharing the same namespace. In my mind an attribute is something completely different than a macro. same logic for fields and methods as in my comment above.

[Centril]

@jchlapinski

@Centril I am not blaming, I am just reasoning on why those things are sharing the same namespace.

Oh; I didn't mean that you are blaming the compiler devs for breaking your code or something, just that I don't think there has been breakage of our stability promises in this case :)

[petrochenkov]

basically we have custom derive for our diagnostic type trait Diag, which is using an fn-like attribute named diag and also a macro (defined with macro_rules) with that same name.

Ah, I see this is an inert attribute white-listed by a derive macro, not an attribute macro.
Then this is indeed a regression affecting stable.

My preferred solution would be to increase priority of derive helper attributes during resolution (item 6 in https://github.com/rust-lang/rust/pull/50911#issuecomment-411605393), rather than introducing more namespaces.

macro diag() {}

#[derive(Diag)] // introduces inert attribute `diag` into scope that shadows `diag`s from outer scopes
#[diag] // refers to the inert attribute
struct S;

[jchlapinski]

@petrochenkov In short, thanks, that solution would solve our problem, and of course it is one way to do it.

But please, if I could take a few more minutes of Your time, explain why You keep referring to attributes, written as #[attribute(...)] as "attribute macros"? Are those really "macros" in rust? A macro is something that gets expanded into code in place it is invoked (with !), exactly as macro_rules defined macros work. An attribute is just an annotation without any inherent influence on the executable code, unless something will use that annotation to generate some code (like for example custom derive). As such those thing (attributes and macros) are completely different and should not shadow one another in any way, regardless whether macros can be expanded in attributes or not. Am I missing something?

[jchlapinski]

@petrochenkov Nevermind, please ignore. The magic name "inert attribute" have provided me with some results to read on. Thanks

[nikomatsakis]

@jchlapinski I know you said ignore, but I'll just expand briefly ;) in short, yes, we are aiming to allow macros to be used in those positions which would edit or alter the struct definition and expand it. #[derive] is one such macro, but it happens to not modify the struct definition but rather just produce extra items alongside (impls, specifically). Think "decorators" in Python, in any case.

[nikomatsakis]

That said, I myself am not entirely clear on the current contours of this design. So perhaps I will pose a few questions that I would like clarified:

• can you actually use a macro-rules macro in this position, or is it an error if a decorator resolves to a macro-rules macro?
  
  
  
  • presumably proc macros can be used
  
  
• can macros beyond derive macros add these "whitelisted attributes"?

[nrc]

do we want all macro_rules and other macros to be usable as macros in attribute position?

No, I don't think so. The two kinds of macros have different 'signatures' so whether decl or proc, they have to be used differently. However, they do share the same namespace.

can you actually use a macro-rules macro in this position, or is it an error if a decorator resolves to a macro-rules macro?

"This position" is item position? In which case it should be fine. I believe with new proc macros there is no distinction between decorators and modifiers, and I don't care either way about old proc macros.

can macros beyond derive macros add these "whitelisted attributes"?

I believe this is currently a special attribute of derive macros. Real proc macros have more power since they consume the input to the macro, they can decide what to allow or disallow.

[petrochenkov]

Fixed in https://github.com/rust-lang/rust/pull/54069

[pnkfelix]

visited for triage. Seems under control from looking at PR #54069. P-high.