Skip to content

Clean rustc_attr_parsing/src/lib.rs documentation #142058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 6, 2025
Merged

Clean rustc_attr_parsing/src/lib.rs documentation #142058

merged 1 commit into from
Jun 6, 2025
+37 −37

Conversation

xizheyin
Copy link
Member

@xizheyin xizheyin commented Jun 5, 2025

Improves the documentation clarity in rustc_attr_parsing by restructuring content with clearer section headers, simplifying explanations of attribute types, making technical descriptions more precise.

r? @oli-obk

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@jdonszelmann jdonszelmann assigned jdonszelmann and unassigned oli-obk Jun 5, 2025
jdonszelmann
jdonszelmann reviewed Jun 5, 2025
View reviewed changes
Copy link
Contributor

@jdonszelmann jdonszelmann left a comment

Choose a reason for hiding this comment

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

Lovely, got a small comment but looks good on the whole @rustbot author

compiler/rustc_attr_parsing/src/lib.rs Outdated
//! - rustc_attr_parsing: this crate
//! - (in the future): rustc_attr_validation
//! This crate is part of a series of crates that handle attribute processing:
//! - rustc_attr_data_structures: Defines the data structures that store parsed attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you point out why there's a split between parsing and data structures?

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2025

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

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 5, 2025
xizheyin
xizheyin commented Jun 5, 2025
View reviewed changes
Copy link
Member Author

@xizheyin xizheyin left a comment

Choose a reason for hiding this comment

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

Thanks for review!
@rustbot ready

compiler/rustc_attr_parsing/src/lib.rs
Comment on lines +9 to +13
//! The separation between data structures and parsing follows the principle of separation of concerns.
//! Data structures (`rustc_attr_data_structures`) define what attributes look like after parsing.
//! This crate (`rustc_attr_parsing`) handles how to convert raw tokens into those structures.
//! This split allows other parts of the compiler to use the data structures without needing
//! the parsing logic, making the codebase more modular and maintainable.
Copy link
Member Author

Choose a reason for hiding this comment

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

I added the explanation in a new commit (not sure if it's right, wrote it intuitively) and opened a new section Architecture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Almost, the important part is that we can't have crate cycles. rustc_hir stores attributes, so has to depend on rustc_attr_data_structures. However, rustc_attr_parsing has to depend on rustc_hir, so if data structures was the same crate as parsing, there would be a cycle.

@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 Jun 5, 2025
@xizheyin
Copy link
Member Author

xizheyin commented Jun 5, 2025

An additional commit to fix typo in rustc_attr_parsing/src/attributes/mod.rs :)

@jdonszelmann
Copy link
Contributor

could you squash them together?

@jdonszelmann
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2025
@xizheyin xizheyin force-pushed the rustc-attr-parsing branch from de36336 to 1e49ad3 Compare June 5, 2025 09:13
@xizheyin
Copy link
Member Author

xizheyin commented Jun 5, 2025

Thanks for the explanation, I didn't know enough about the dependencies between the various components of rustc. I've been working on this module lately for the implementation of #141617 and I hope everything goes well.

I have squashed the commits. @rustbot ready

@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 Jun 5, 2025
@jdonszelmann
Copy link
Contributor

jdonszelmann commented Jun 5, 2025 •
edited
Loading

I see! no worries, I'm the person actively working on this refactor so feel free to ask my review for work around here with r? @jdonszelmann whenever you need it. @bors r+ rollup

@rustbot

This comment was marked as resolved.

Sign in to view
@bors
Copy link
Collaborator

bors commented Jun 5, 2025

📌 Commit 1e49ad3 has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors 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 Jun 5, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 5, 2025
@GuillaumeGomez
Rollup merge of rust-lang#142058 - xizheyin:rustc-attr-parsing, r=jdo…
fccd4a1 
…nszelmann

Clean `rustc_attr_parsing/src/lib.rs` documentation

Improves the documentation clarity in `rustc_attr_parsing` by restructuring content with clearer section headers, simplifying explanations of attribute types, making technical descriptions more precise.

r? `@oli-obk`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jun 5, 2025
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 5, 2025
Merged
bors added a commit that referenced this pull request Jun 6, 2025
@bors
Auto merge of #142099 - matthiaskrgr:rollup-r9s3c35, r=matthiaskrgr
2082a82 
Rollup of 11 pull requests

Successful merges:

 - #125087 (Optimize `Seek::stream_len` impl for `File`)
 - #141982 (`tests/ui`: A New Order [5/N])
 - #142012 (Replace some `Option<Span>` with `Span` and use DUMMY_SP instead of None)
 - #142044 (compiler: Document the offset invariant of `OperandValue::Pair`)
 - #142047 (Ensure stack in two places that affect s390x)
 - #142058 (Clean `rustc_attr_parsing/src/lib.rs` documentation)
 - #142067 (canon_abi: make to_erased_extern_abi just a detail in formatting)
 - #142072 (doc: Fix inverted meaning in E0783.md)
 - #142084 (add myself to rotation)
 - #142091 (Fix AIX build)
 - #142092 (rustdoc: Support middle::ty associated const equality predicates again)

Failed merges:

 - #142042 (Make E0621 missing lifetime suggestion verbose)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jun 6, 2025
@bors
Auto merge of #142099 - matthiaskrgr:rollup-r9s3c35, r=matthiaskrgr
16ca0cb 
Rollup of 11 pull requests

Successful merges:

 - #125087 (Optimize `Seek::stream_len` impl for `File`)
 - #141982 (`tests/ui`: A New Order [5/N])
 - #142012 (Replace some `Option<Span>` with `Span` and use DUMMY_SP instead of None)
 - #142044 (compiler: Document the offset invariant of `OperandValue::Pair`)
 - #142047 (Ensure stack in two places that affect s390x)
 - #142058 (Clean `rustc_attr_parsing/src/lib.rs` documentation)
 - #142067 (canon_abi: make to_erased_extern_abi just a detail in formatting)
 - #142072 (doc: Fix inverted meaning in E0783.md)
 - #142084 (add myself to rotation)
 - #142091 (Fix AIX build)
 - #142092 (rustdoc: Support middle::ty associated const equality predicates again)

Failed merges:

 - #142042 (Make E0621 missing lifetime suggestion verbose)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jun 6, 2025
@bors
Auto merge of #142099 - matthiaskrgr:rollup-r9s3c35, r=matthiaskrgr
f315e61 
Rollup of 11 pull requests

Successful merges:

 - #125087 (Optimize `Seek::stream_len` impl for `File`)
 - #141982 (`tests/ui`: A New Order [5/N])
 - #142012 (Replace some `Option<Span>` with `Span` and use DUMMY_SP instead of None)
 - #142044 (compiler: Document the offset invariant of `OperandValue::Pair`)
 - #142047 (Ensure stack in two places that affect s390x)
 - #142058 (Clean `rustc_attr_parsing/src/lib.rs` documentation)
 - #142067 (canon_abi: make to_erased_extern_abi just a detail in formatting)
 - #142072 (doc: Fix inverted meaning in E0783.md)
 - #142084 (add myself to rotation)
 - #142091 (Fix AIX build)
 - #142092 (rustdoc: Support middle::ty associated const equality predicates again)

Failed merges:

 - #142042 (Make E0621 missing lifetime suggestion verbose)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7420ae8 into rust-lang:master Jun 6, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 6, 2025
rust-timer added a commit that referenced this pull request Jun 6, 2025
@rust-timer
Unrolled build for #142058
52fdd7d 
Rollup merge of #142058 - xizheyin:rustc-attr-parsing, r=jdonszelmann

Clean `rustc_attr_parsing/src/lib.rs` documentation

Improves the documentation clarity in `rustc_attr_parsing` by restructuring content with clearer section headers, simplifying explanations of attribute types, making technical descriptions more precise.

r? ``@oli-obk``
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 7, 2025
@bors
Auto merge of #142099 - matthiaskrgr:rollup-r9s3c35, r=matthiaskrgr
d6ea7d2 
Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#125087 (Optimize `Seek::stream_len` impl for `File`)
 - rust-lang/rust#141982 (`tests/ui`: A New Order [5/N])
 - rust-lang/rust#142012 (Replace some `Option<Span>` with `Span` and use DUMMY_SP instead of None)
 - rust-lang/rust#142044 (compiler: Document the offset invariant of `OperandValue::Pair`)
 - rust-lang/rust#142047 (Ensure stack in two places that affect s390x)
 - rust-lang/rust#142058 (Clean `rustc_attr_parsing/src/lib.rs` documentation)
 - rust-lang/rust#142067 (canon_abi: make to_erased_extern_abi just a detail in formatting)
 - rust-lang/rust#142072 (doc: Fix inverted meaning in E0783.md)
 - rust-lang/rust#142084 (add myself to rotation)
 - rust-lang/rust#142091 (Fix AIX build)
 - rust-lang/rust#142092 (rustdoc: Support middle::ty associated const equality predicates again)

Failed merges:

 - rust-lang/rust#142042 (Make E0621 missing lifetime suggestion verbose)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdonszelmann jdonszelmann jdonszelmann left review comments

Assignees

@jdonszelmann jdonszelmann

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
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
5 participants
@xizheyin @rustbot @jdonszelmann @bors @oli-obk