Disentangle AST crates and error crates#155237
Open
nnethercote wants to merge 4 commits intorust-lang:mainfrom
Open
Disentangle AST crates and error crates#155237nnethercote wants to merge 4 commits intorust-lang:mainfrom
nnethercote wants to merge 4 commits intorust-lang:mainfrom
Conversation
Collaborator
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_codegen_gcc
cc @TaKO8Ki |
Contributor
Author
This comment has been minimized.
This comment has been minimized.
Contributor
|
@nnethercote that crate graph visualization looks nice, what tool did you use to generate it? |
Contributor
Author
Requires installing cargo-depgraph. |
`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.
rustc_ast* dependencies696f8d9 to
6ee5111
Compare
This comment has been minimized.
This comment has been minimized.
`rustc_hir_id` is a very small crate containing some basic HIR types (e.g. `HirId`) so that other crates can use them without depending on `rustc_hir` (which is a much bigger crate). However, `rustc_span` already has a module called `def_id` which also contains some basic HIR types (e.g. `DefId`). So there's not much point also have `rustc_hir_id`. This commit moves the contents of `rustc_hir_id` into that module.
This puts it alongside `rustc_span::def_id`, which serves a similar purpose. This lets us remove `rustc_errors`'s dependency on `rustc_ast`, completing the disentangling of the AST crates and the error crates.
6ee5111 to
9b693c1
Compare
Contributor
Author
|
@davidtwco: this is now ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Currently the
rustc_ast*crates and therustc_error*crates (andrustc_lint_defs) are quite intertwined. This PR disentangles them. Details in individual commits.r? @davidtwco