Skip to content

Disentangle AST crates and error crates#155237

Merged
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
nnethercote:ast-errors-rejig
May 1, 2026
Merged

Disentangle AST crates and error crates#155237
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
nnethercote:ast-errors-rejig

Conversation

@nnethercote
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote commented Apr 13, 2026

View all comments

Currently the rustc_ast* crates and the rustc_error* crates (and rustc_lint_defs) are quite intertwined. This PR disentangles them. Details in individual commits.

r? @davidtwco

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 13, 2026

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

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 in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

rustc_error_messages was changed

cc @TaKO8Ki

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 13, 2026
@nnethercote
Copy link
Copy Markdown
Contributor Author

nnethercote commented Apr 13, 2026

The path from rustc_hir to rustc_span, before:
image

And after:
image

I think the latter is a lot nicer -- much less linear, with rustc_hir_id gone and rustc_error* fully separated from rustc_ast*.

@rust-log-analyzer

This comment has been minimized.

@nnethercote nnethercote marked this pull request as draft April 13, 2026 12:43
@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 Apr 13, 2026
@nik-rev
Copy link
Copy Markdown
Contributor

nik-rev commented Apr 13, 2026

@nnethercote that crate graph visualization looks nice, what tool did you use to generate it?

@nnethercote
Copy link
Copy Markdown
Contributor Author

@nik-rev:

cargo +nightly depgraph --all-deps --dedup-transitive-deps --workspace-only --root=rustc-main,rustc_codegen_llvm ~/graph.dot
dot -Tsvg ~/graph.dot > ~/graph.svg

Requires installing cargo-depgraph.

@nnethercote nnethercote changed the title Adjust rustc_ast* dependencies Distentangle AST crates and error crates Apr 14, 2026
@nnethercote nnethercote changed the title Distentangle AST crates and error crates Disentangle AST crates and error crates Apr 14, 2026
@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 14, 2026
@rust-log-analyzer

This comment has been minimized.

@nnethercote nnethercote marked this pull request as ready for review April 14, 2026 10:25
@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 14, 2026
@nnethercote
Copy link
Copy Markdown
Contributor Author

@davidtwco: this is now ready for review.

@rust-bors

This comment has been minimized.

@rustbot

This comment has been minimized.

@nnethercote
Copy link
Copy Markdown
Contributor Author

I rebased.

@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Apr 25, 2026

I think, ideally, generic linting APIs living in crates like rustc_error_messages should not depend on concrete "lint reporting position" types like HirId and NodeId at all, especially not HirId.
They should ideally use some abstract "LintNodeId" APIs instead, which would be filled with concrete HIR/AST ids when HIR and AST respectively are available.

That would untangle some users of the generic linting infra, like rustc_attr_parsing from HIR ids as well.
(Because the Attribute IR is not HIR, should not contain HirIds or any other HIR parts, should not live in rustc_hir, and rusc_attr_parsing should not depend on rustc_hir as well cc @JonathanBrouwer)
(Untangling it from NodeIds is less important, because attr parsing definitely needs token streams, and they currently live in rustc_ast despite not being AST, and rustc_ast defines NodeId which is AST. Although it could be nice to try doing some day in the future.)

That's why I don't like that HirId and NodeId are moved to rustc_span so they are available in rustc_error_messages more easily.
Although I understand that reworking the linting APIs would be a much more involved change, so I don't want to block this PR which should be a clear improvement to the compiler build times.

However, rustc_span already has a module called def_id which also contains some basic HIR types (e.g. DefId)

I wouldn't classify DefId as HIR or any other IR, it's more basic infra, and it's actively used starting from early macro expansion, when even AST is not fully built.

@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Apr 25, 2026

That's why I don't like that HirId and NodeId are moved to rustc_span so they are available in rustc_error_messages more easily.

Hmmm, perhaps their definitions can be moved to rustc_error_messages itself, where they are first needed currently?
Maybe accompanied with a FIXME for reworking the linting APIs.
That would make it clear that something is not right at the moment.
Then they'd be reexported from their "canonical" locations in rustc_ast and rustc_hir.

@nnethercote
Copy link
Copy Markdown
Contributor Author

Hmmm, perhaps their definitions can be moved to rustc_error_messages itself, where they are first needed currently?

HirId and NodeId aren't used in rustc_error_messages. NodeId is used in rustc_errors. The nice thing about having NodeId in rustc_span is that the ast crates and the error crates all depend on rustc_span but otherwise don't have any interdependencies.

@petrochenkov
Copy link
Copy Markdown
Contributor

Hmmm, perhaps their definitions can be moved to rustc_error_messages itself, where they are first needed currently?

HirId and NodeId aren't used in rustc_error_messages. NodeId is used in rustc_errors. The nice thing about having NodeId in rustc_span is that the ast crates and the error crates all depend on rustc_span but otherwise don't have any interdependencies.

So I checked again what depends on what, and this my suggestion would then translate to:

  • move rustc_hir_id stuff into rustc_lint_defs with a fixme saying that rustc_lint_defs shouldn't ideally depend on it and that it should live in rustc_hir.
  • move node_id to a separate crate rustc_node_id with a fixme saying that rustc_errors shouldn't ideally depend on it and it should live in rustc_ast.

The first two commits are definitely good to go.

@petrochenkov petrochenkov 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 Apr 29, 2026
@rustbot

This comment has been minimized.

@nnethercote
Copy link
Copy Markdown
Contributor Author

nnethercote commented Apr 30, 2026

  • move rustc_hir_id stuff into rustc_lint_defs with a fixme saying that rustc_lint_defs shouldn't ideally depend on it and that it should live in rustc_hir.

  • move node_id to a separate crate rustc_node_id with a fixme saying that rustc_errors shouldn't ideally depend on it and it should live in rustc_ast.

The first two commits are definitely good to go.

I'll just merge the first two commits; they are the more important ones. I don't want to move rustc_hir_id and node_id only to leave behind FIXMEs saying that they should move somewhere else.

@bors r=petrochenkov

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 30, 2026

📌 Commit 1509578 has been approved by petrochenkov

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 30, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Apr 30, 2026
…trochenkov

Disentangle AST crates and error crates

Currently the `rustc_ast*` crates and the `rustc_error*` crates (and `rustc_lint_defs`) are quite intertwined. This PR disentangles them. Details in individual commits.

r? @davidtwco
rust-bors Bot pushed a commit that referenced this pull request Apr 30, 2026
Rollup of 6 pull requests

Successful merges:

 - #155853 (Use `_mcount` as the mcount symbol name on RISC-V Linux GNU targets)
 - #155939 (Add feature gate for view_types experiment)
 - #155974 (add `c_variadic_experimental_arch` feature)
 - #155523 (Reorganize `tests/ui/issues/` - 02)
 - #155980 (Move `feature*` methods from `parse` mod to `errors` mod.)
 - #155987 (Make lifting infallible)

Failed merges:

 - #155237 (Disentangle AST crates and error crates)
@rust-bors rust-bors Bot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 30, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors Bot removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 30, 2026
`rustc_error_messages` currently depends on
`rustc_ast`/`rustc_ast_pretty`. This is odd, because
`rustc_error_messages` feels like a very low-level module but
`rustc_ast`/`rustc_ast_pretty` do not.

The reason is that a few AST types impl `IntoDiagArg` via
pretty-printing. `rustc_error_messages` can define `IntoDiagArg` and
then impl it for the AST types. But if we invert the dependency we hit
a problem with the orphan rule: `rustc_ast` must impl `IntoDiagArg`
for the AST types, but that requires calling pretty-printing code which
is in `rustc_ast_pretty`, a downstream crate.

This commit avoids this problem by just removing the `IntoDiagArg` impls
for these AST types. There aren't that many of them, and we can just use
`String` in the relevant error structs and use the pretty printer in the
downstream crates that construct the error structs. There are plenty of
existing examples where `String` is used in error structs.

There is now no dependency between `rustc_ast*` and
`rustc_error_messages`.
It currently only depends on two things:
- `rustc_ast::AttrId`: this is just a re-export of `rustc_span::AttrId`,
  so we can import the original instead.
- `rustc_ast::AttributeExt`: needed only for the `name` and `id`
  methods. We can instead pass in the `name` and `id` directly.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 1, 2026

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.

@nnethercote
Copy link
Copy Markdown
Contributor Author

I rebased.

@bors r=petrochenkov

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 1, 2026

📌 Commit 339797e has been approved by petrochenkov

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 1, 2026
rust-bors Bot pushed a commit that referenced this pull request May 1, 2026
Rollup of 19 pull requests

Successful merges:

 - #155237 (Disentangle AST crates and error crates)
 - #155249 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup)
 - #155853 (Use `_mcount` as the mcount symbol name on RISC-V Linux GNU targets)
 - #155919 (simplify `ast_fragments!`)
 - #155939 (Add feature gate for view_types experiment)
 - #155954 (rustdoc: preserve parent doc cfg for `macro_export` macros)
 - #155974 (add `c_variadic_experimental_arch` feature)
 - #155991 (Catch unwinds from the global ctxt callback to complete queries profiling data in more cases)
 - #156003 (Pass Session to optimize_and_codegen_fat_lto)
 - #153566 (Add suggestion for E0401 on inner const items)
 - #154610 (Suggest public re-exports when a private module makes an import path inaccessible)
 - #155523 (Reorganize `tests/ui/issues/` - 02)
 - #155821 (c-variadic: document `Clone` and `Drop` instances and require `VaArgSafe: Copy`)
 - #155980 (Move `feature*` methods from `parse` mod to `errors` mod.)
 - #155987 (Make lifting infallible)
 - #155988 (tests/run-make/print-cfg: add Android target_env case)
 - #156000 (Fix ICE when using -Zinstrument-mcount and -Clinker-flavor=lld)
 - #156002 (Allow to use `Diagnostic` directly in `SharedContext::emit_lint`)
 - #156015 (rustc-dev-guide subtree update)
@rust-bors rust-bors Bot merged commit f5304d2 into rust-lang:main May 1, 2026
11 checks passed
@rustbot rustbot added this to the 1.97.0 milestone May 1, 2026
rust-timer added a commit that referenced this pull request May 1, 2026
Rollup merge of #155237 - nnethercote:ast-errors-rejig, r=petrochenkov

Disentangle AST crates and error crates

Currently the `rustc_ast*` crates and the `rustc_error*` crates (and `rustc_lint_defs`) are quite intertwined. This PR disentangles them. Details in individual commits.

r? @davidtwco
@nnethercote nnethercote deleted the ast-errors-rejig branch May 1, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-clippy Relevant to the Clippy team. 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.

6 participants