Skip to content

Add a higher-level API for parsing attributes#155696

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
scrabsha:push-kxqstpltlwzn
Apr 26, 2026
Merged

Add a higher-level API for parsing attributes#155696
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
scrabsha:push-kxqstpltlwzn

Conversation

@scrabsha

@scrabsha scrabsha commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

I got quite frustrated that a lot of the attribute parsing API repeats the same pattern over and over:

let Some(list) = args.list() else {
    cx.expected_list(args, span);
    return None;
}

(Example uses list, but this happens to most of the methods in rustc_attr_parsing::parser actually)

Besides the repetitions, here's why I don't like it:

  • It separates the checks on the attribute arguments with the error emission
  • Early returns are explicit and take a lot of space
  • It is quite easy to accidentally generate an error message that has little to do with the check that has just been done (I already fixed things in 1, 2)
  • I am convinced that APIs that require too much boilerplate and repetitions give people an incentive to use Automated Code Generation Tools ™️ and I would like to avoid giving this incentive.

This PR adds an AcceptContext::expect_list method, allowing to replace the code in the previous snippet with:

let list = cx.expect_list(args, span)?;

which I believe solves the concerns I wrote earlier.

I already introduced an AcceptContext::expect_single_element_list method that follows similar style in #154827, and ideally I would like this PR to generalize this idea.

I do not expect this higher-level API to replace the methods in rustc_attr_parsing::parser, but rather to provide a higher-level version that can be used in most simple situations. This is explained both in the documentation added in this PR and in #154827 (comment).

In the long term, I also hope we could make AcceptContext::expect_list and friends push custom suggestions to the diagnostics and hopefully stop relying on the attribute template in future.

The first commit introduces AcceptContext::expect_list and AcceptContext::expect_single and uses it where it makes sense to. I added only these two because I figured it would be a nice way to ask for feedback without investing too much time. The second commit adds documentation for the newly added methods.

@JonathanBrouwer I think you're the best person to ask feedback from, let me know what you think!


I also tried to make the parsing functions return a Result<Self, ErrorGuaranteed>, but I don't think it is possible due to Combine::finalize

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 23, 2026
@JonathanBrouwer JonathanBrouwer self-assigned this Apr 23, 2026
@rust-log-analyzer

This comment has been minimized.

Comment thread compiler/rustc_attr_parsing/src/context.rs Outdated
Comment thread compiler/rustc_attr_parsing/src/context.rs
Comment thread compiler/rustc_attr_parsing/src/context.rs Outdated
Comment thread compiler/rustc_attr_parsing/src/context.rs Outdated
@rustbot

rustbot commented Apr 23, 2026

Copy link
Copy Markdown
Collaborator

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

@scrabsha
scrabsha force-pushed the push-kxqstpltlwzn branch from 5c92c1e to 067ef3d Compare April 24, 2026 15:55
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2026
@scrabsha
scrabsha marked this pull request as ready for review April 24, 2026 15:56
@rustbot

rustbot commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred to diagnostic attributes.

cc @mejrs

Comment on lines +258 to 271
// FIXME(scrabsha): once #155696 is merged, update this and mention the higher-level APIs.
/// Utility that deconstructs a MetaItem into usable parts.
///
/// MetaItems are syntactically extremely flexible, but specific attributes want to parse
/// them in custom, more restricted ways. This can be done using this struct.
///
/// MetaItems consist of some path, and some args. The args could be empty. In other words:
///
/// - `name` -> args are empty
/// - `name(...)` -> args are a [`list`](ArgParser::list), which is the bit between the parentheses
/// - `name(...)` -> args are a [`list`](ArgParser::as_list), which is the bit between the parentheses
/// - `name = value`-> arg is [`name_value`](ArgParser::name_value), where the argument is the
/// `= value` part
///
/// The syntax of MetaItems can be found at <https://doc.rust-lang.org/reference/attributes.html>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It felt wrong to mention AcceptContext::expect_list/AcceptContext::expect_single here given that the other functions don't exist yet. If that's good for you, I'll update this doc comment once all the AcceptContext::expect_* functions are implemented.

@scrabsha

scrabsha commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

@JonathanBrouwer so this PR renames list -> as_list and single -> as_single. should I immediately rename every method in parser.rs or is it ok if I only rename the other methods as I implement their replacement?

(I don't really have an opinion on what is better, it's up to you ^^)

@JonathanBrouwer JonathanBrouwer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rust-bors

rust-bors Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 067ef3d has been approved by JonathanBrouwer

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 25, 2026
…thanBrouwer

Add a higher-level API for parsing attributes
rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #154803 (Fix ICE from cfg_attr_trace )
 - #155485 (Add an edge-case test for `--remap-path-prefix` for `rustc` & `rustdoc`)
 - #155659 (cleanup, restructure and merge `tests/ui/deriving` into `tests/ui/derives`)
 - #155696 (Add a higher-level API for parsing attributes)
 - #155734 (Lint doc comments in cfg_select branches)
 - #155769 (triagebot.toml: Ping Enselic when tests/debuginfo/basic-stepping.rs changes)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 25, 2026
…thanBrouwer

Add a higher-level API for parsing attributes
rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #154803 (Fix ICE from cfg_attr_trace )
 - #155485 (Add an edge-case test for `--remap-path-prefix` for `rustc` & `rustdoc`)
 - #155659 (cleanup, restructure and merge `tests/ui/deriving` into `tests/ui/derives`)
 - #155696 (Add a higher-level API for parsing attributes)
 - #155769 (triagebot.toml: Ping Enselic when tests/debuginfo/basic-stepping.rs changes)
rust-bors Bot pushed a commit that referenced this pull request Apr 25, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #146181 (Add intrinsic for launch-sized workgroup memory on GPUs)
 - #154803 (Fix ICE from cfg_attr_trace )
 - #155065 (Error on invalid macho section specifier)
 - #155485 (Add an edge-case test for `--remap-path-prefix` for `rustc` & `rustdoc`)
 - #155659 (cleanup, restructure and merge `tests/ui/deriving` into `tests/ui/derives`)
 - #155676 ( Reject implementing const Drop for types that are not const `Destruct` already)
 - #155696 (Add a higher-level API for parsing attributes)
 - #155769 (triagebot.toml: Ping Enselic when tests/debuginfo/basic-stepping.rs changes)
 - #155783 (Do not suggest internal cfg trace attributes)
@rust-bors
rust-bors Bot merged commit 97dd613 into rust-lang:main Apr 26, 2026
11 checks passed
@rustbot rustbot added this to the 1.97.0 milestone Apr 26, 2026
rust-timer added a commit that referenced this pull request Apr 26, 2026
Rollup merge of #155696 - scrabsha:push-kxqstpltlwzn, r=JonathanBrouwer

Add a higher-level API for parsing attributes
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 13, 2026
doc: clarify attr parser APIs

Follow up on rust-lang#155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 13, 2026
doc: clarify attr parser APIs

Follow up on rust-lang#155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 13, 2026
doc: clarify attr parser APIs

Follow up on rust-lang#155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 13, 2026
doc: clarify attr parser APIs

Follow up on rust-lang#155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
rust-timer added a commit that referenced this pull request Jul 13, 2026
Rollup merge of #159123 - kn1g78:doc/attr-parser-api, r=mejrs

doc: clarify attr parser APIs

Follow up on #155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
github-actions Bot pushed a commit to rust-lang/stdarch that referenced this pull request Jul 16, 2026
doc: clarify attr parser APIs

Follow up on rust-lang/rust#155696 by documenting the higher-level `AcceptContext` helpers for common argument shapes.

`MetaItemParser` remains the lower-level tool for attributes with custom restricted syntax. Remove the stale FIXME now that the helpers are available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants