Warn or error on duplicate attributes

[jdonszelmann]

During the port of built-in attributes we've found that some attributes give an error if repeated, some a warning, and some a "future error". i.e. warning for now saying that some day it's going to be an error.

@JonathanBrouwer started changing some to used with future error, and I think that's not unreasonable. For example, #[used] will under his PR error when used multiple times. See #142818 (comment), #142818 (comment) and #142823 (comment)

There is some value in allowing duplicates. For example, an attribute macro might emit #[used] and also #[allow(duplicate_attributes)] to make a function used regardless of a user making already having marked it as used. That saves the attribute macro from checking if the attribute is already there. Is that a good usecase? I don't know. However, since we currently do have all three options, I thought it might be good to open a discussion on what we should actually want, in general.

Some comments were made on zulip already. The questions to be answered are:

• Do we want the attributes that warn now to keep warning or to promise future error
• Attributes that promise future error now, when is that future? Do we actually want to make them an error or instead downgrade them to just a warning? See also A path towards erroring on nonsense attributes #142838
• Can we even come up with a policy or is it inherently attribute-dependent

And, if answers to those questions are complicated, what policy should we maintain for future attributes. Should list-like attributes always allow duplicates? Also if they're nested?

Some examples of the current state before the refactorings in progress. The refactorings can maintain the status quo, but maybe we want to formalize that.

duplicates obviously allowed

#[repr(C)]
#[repr(align(8))]
struct Foo;

duplicates allowed with warning of future error

#[inline(always)]
#[inline(never)] //~ WARN will be an error in the future
#[inline] //~ WARN will be an error in the future
fn foo() {}

duplicates allowed with a warning

#[cold]
#[cold] //~ WARN unused attribute
fn foo() {}

// NOTE: we might want to make this an error or warn-future-error? 
// but again, we have no policy why this is different, or not different than inline for example
#[used(compiler)]
#[used(linker)] // WARN unused attribute
fn bar() {}

duplicates disallowed

This one seems obvious

#[deprecated = "a"]
#[deprecated = "b"] //~ ERROR duplicate
fn foo() {}

This one also, sort of, though the keys are disjoint so in theory not much wrong with it.

#[deprecated(since="a")]
#[deprecated(note = "reason")] //~ ERROR duplicate
fn foo() {}

But this one less. It seems like splitting this over two attributes is not so different from repr for example.

#[proc_macro_derive(Foo, attributes(a, b, c))]
#[proc_macro_derive(attributes(e, f, g))] //~ ERORR duplicate
fn foo() {}

Similarly,

#[collapse_debuginfo(yes)]
#[collapse_debuginfo(external)]//~ ERORR duplicate
fn foo() {}

is not so different from inline or used, while this is a hard error and those two are future error or warning respectively.

Finally, there is also no real consensus which attribute takes precedence, the last or the first for built-in attributes. That one seems harder to fix since it might break people's code. It's easy to keep it as is, though we could think about changing that over an edition. Food for thought.

cc: @mejrs @JonathanBrouwer @oli-obk @bjorn3