Remove ATTRIBUTE_ORDER

[JonathanBrouwer]

View all comments

r? @jdonszelmann

There's only a few attributes that use KeepInnermost and I think I like the consistency of just making them all KeepOutermost

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

[JonathanBrouwer]

This order was inconsistent with the order in Warn and WarnButFutureError

[jdonszelmann]

Aren't these technically breaking changes?

[JonathanBrouwer]

Why is this a breaking change? I think erroring/warning on a different attribute should be fine right?

[jdonszelmann]

Well in the cases where it warns, it compiles. Let's say there are two inline attributes, one says always the other never. It will warn about the innermost, apply the outermost, and be inlined. If we make the attribute keep the innermost, it warns about the outermost and not inline

[JonathanBrouwer]

Ooh right, yeah you're right, good catch.
I'll think about this a little bit but will probably just submit it to T-lang

[JonathanBrouwer]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 54bb9bb (54bb9bb1bbe87f59b5a2e9afea4c8cedcf5e0b47, parent: 0028f344ce9f64766259577c998a1959ca1f6a0b)

[JonathanBrouwer]

@craterbot check

[craterbot]

👌 Experiment pr-153041 created and queued.
🤖 Automatically detected try build 54bb9bb
⚠️ Try build based on commit 6bafe7f, but latest commit is 698abd4. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-153041 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[craterbot]

🎉 Experiment pr-153041 is completed!
📊 4 regressed and 3 fixed (827089 total)
📊 2463 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-153041/retry-regressed-list.txt ↩

[scottmcm]

I would also be inclined to bump the lint to deny-by-default as part of it, since if there's code outside crater that does have this, I think it'd probably be better to force them to look at it and allow it if it might have changed behaviour.

[traviscross]

Sounds right. Thanks @JonathanBrouwer.

@rfcbot fcp merge lang

@JonathanBrouwer: What's the story on the accompanying linting? We were talking about this in the meeting. What's the most we could do to flag this to people while not breaking any legitimate use cases, e.g., macros?

[joshtriplett]

👍 for trying to catch obvious cases with a lint, but there may be legitimate instances of intentionally wanting to override a macro-specified attribute with a different value.

[joshtriplett]

@rfcbot reviewed

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[jdonszelmann]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[traviscross]

We need to update the Reference for this. Note that two of these touch outstanding PRs:

• Update export_name to use the attribute template reference#1902
• Update link_section to use the attribute template reference#1903

cc @rust-lang/lang-docs @rust-lang/fls

[JonathanBrouwer]

Left comments on the two mentioned PRs and made rust-lang/reference#2213
@jdonszelmann this should now be ready
@rustbot ready