Merged RustcForceInline & Inline Attribute Parsers

[tahadostifam]

View all comments

Merged RustcForceInline & Inline Attribute Parsers

I've been reading #153101, #131229 and I came across with this comment in compiler/rustc_attr_parsing/src/attributes/inline.rs:

// FIXME(jdonszelmann): merge these two parsers and error when both attributes are present here.
//                      note: need to model better how duplicate attr errors work when not using
//                      SingleAttributeParser which is what we have two of here.

Having separate SingleAttributeParser implementations for #[rustc_force_inline] and #[inline(...)] meant the compiler wasn't able to recognize cases where both of them were used together on the same item, for example:

#![feature(rustc_attrs)]

#[rustc_force_inline]
#[inline]
fn foo() {}

fn main() {}

This case was previously considered allowed, which is not correct.

My changes replace the distinct attribute parsers with a single unified AttributeParser implementation for InlineParser, handling both attributes together in a single pass. By consolidating the logic, the parser now tracks the state of both attributes side-by-side using an AcceptMapping and introduces a new session diagnostic, InlineForceInlineConflict, which is explicitly triggered in the finalize step if a user attempts to combine them.

An added benefit of catching this conflict early during the parsing phase is that it prevents downstream validation passes (like check_attr) from triggering redundant errors on malformed or incorrectly placed attributes, which naturally cleans up and streamlines our compiler stderr output as reflected in the updated UI test baselines.

This is my very first PR on rustc, and I am incredibly grateful to @hkalbasi, who heavily guided me through the codebase and helped make this change possible!

Sincerely looking for your feedback and thoughts on this, let me know if there is something need to be changed.

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @folkertdev (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 22 candidates

[JonathanBrouwer]

(would like to take a look at this, this might take me a few days to get to, should have time to take a look on friday)

[JonathanBrouwer]

Haven't reviewed the full diff yet, will do so after these two comments are fixed, thanks :3

View changes since this review

[JonathanBrouwer]

This change is not correct, always does not take any arguments.
This error would suggest always(always) would be valid.
Please drop that commit

[JonathanBrouwer]

You made changes to tests that are not related to this PR, could you at the very least split these changes into individual commits, and preferably into separate PRs? It makes the diff hard to review

[rustbot]

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

[tahadostifam]

Thanks for your feedback, I've applied the new changes and removed irrelevant ones.

#[inline(always = 5)]
fn foo() {}

This now emits unexpected_args as you said:

error[E0565]: malformed `inline` attribute input
  --> /tmp/main.rs:14:1
   |
14 | #[inline(always = 5)]
   | ^^^^^^^^^----------^^
   |          |
   |          didn't expect any arguments here

Currently with rustc 1.98-nightly this compiles with no error:

#[inline(always = 1)]
#[inline(always = "a")]
fn foo() {}

#[inline(always = 1)]
fn bar() {}

#[inline(always = "b")]
fn baz() {}

#[inline(always(always))]
fn buz() {}

fn main() {}

(Only generates warning but compiles successfully)

All of those malformed cases are covered now, with my changes.

Also reviewed spans of diagnostics several times, but if there is still something that must be changed, let me know.

[tahadostifam]

I'm still waiting for a review.

[JonathanBrouwer]

Yep sorry, have been quite busy with moving recently, will get to it soon

[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.

[tahadostifam]

Docker registry got timeout error, maybe it'd be fixed if we run the workflow again.
(I thought reroll command is going to rerun worfklows, but turned out something else though I'm not familiar with this bot, Please forgive the mess of many comments here.)

[tahadostifam]

@rustbot assign @JonathanBrouwer

[rustbot]

JonathanBrouwer is currently at their maximum review capacity.
They may take a while to respond.

[tahadostifam]

This PR is ready to be merged now. @JonathanBrouwer