rust-lang / rust-lang/rust

Warn or error on duplicate attributes

Open
#142,836 12 comments 6 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-attributes I-lang-radar T-lang
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

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 https://github.com/rust-lang/rust/pull/142818#discussion_r2160010274, https://github.com/rust-lang/rust/pull/142818#discussion_r2159982637 and https://github.com/rust-lang/rust/pull/142823#discussion_r2160015573

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 https://github.com/rust-lang/rust/issues/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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the referenced PR discussions, the Zulip thread, and the duplicate-attribute examples in this issue. Compare the current warning, future-error, and hard-error cases, including precedence and nested or list-like attributes. Done means reaching and documenting a consistent policy for existing and future built-in attributes.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.