Skip to content
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.

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

Split up attribute parsing code and move data types to rustc_attr_data_structures #134381

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jdonszelmann
Copy link
Contributor

@jdonszelmann jdonszelmann commented Dec 16, 2024

This change renames rustc_attr to rustc_attr_parsing, and splits up the parsing code. At the same time, all the data types used move to rustc_attr_data_structures. This is in preparation of also having a third crate: rustc_attr_validation

I initially envisioned this as two separate PRs, but I think doing it in one go reduces the number of ways others would have to rebase their changes on this. However, I can still split them.

r? @oli-obk (we already discussed how this is a first step in a larger plan)

For a more detailed plan on how attributes are going to change, see #131229

Edit: this looks like a giant PR, but the changes are actually rather trivial. Each commit is reviewable on its own, and mostly moves code around. No new logic is added.

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 16, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@jdonszelmann
Copy link
Contributor Author

@rustbot label +A-attributes

For the rustdoc changes, I only made that code depend on rustc_attr_parsing instead of rustc_attr. No actual logic changed.

@rustbot rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Dec 16, 2024
@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
Copy link
Contributor Author

jdonszelmann commented Dec 16, 2024

well, my last rebase had no conflicts, but clearly it didn't work very well anyway... 1s

@jdonszelmann jdonszelmann force-pushed the move-attribute-types branch 3 times, most recently from 7179f3b to ff646ad Compare December 16, 2024 11:43
@rust-log-analyzer

This comment has been minimized.

a.out Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

accidentally committed a test file

Copy link
Contributor

Choose a reason for hiding this comment

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

(even though it was removed in the second commit)

Copy link
Contributor Author

@jdonszelmann jdonszelmann Dec 16, 2024

Choose a reason for hiding this comment

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

oops, yea I had noticed but should've removed it from the first one haha

@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 Dec 16, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Dec 16, 2024

I reviewed some changes and the overall change. I'm just gonna assume all the individual parts have been moved correctly (as no tests have been touched, and the downstream usages are all just import changes, this appears so)

@bors delegate+

r=me with the nit for a clean history

@bors
Copy link
Contributor

bors commented Dec 16, 2024

✌️ @jdonszelmann, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@jdonszelmann
Copy link
Contributor Author

@bors r=@oli-obk

@bors
Copy link
Contributor

bors commented Dec 16, 2024

📌 Commit efb98b6 has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 16, 2024
@jieyouxu
Copy link
Member

@bors p=6 (conflict-prone)

@jieyouxu
Copy link
Member

(conflict-check)

@jieyouxu jieyouxu closed this Dec 17, 2024
@jieyouxu jieyouxu reopened this Dec 17, 2024
@jieyouxu
Copy link
Member

@bors r=@oli-obk

@bors

This comment was marked as resolved.

@bors
Copy link
Contributor

bors commented Dec 17, 2024

📌 Commit efb98b6 has been approved by oli-obk

It is now in the queue for this repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-json Area: Rustdoc JSON backend 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants