-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Remove implicit dependencies on syntax::pprust #65363
Conversation
Instead just use `pprust::path_to_string(..)` where needed. This has two benefits: a) The AST definition is now independent of printing it. (Therefore we get closer to extracting a data-crate.) b) Debugging should be easier as program flow is clearer.
Historically I've looked at doing this but didn't see it as immediately actionable. Seems like pprust is still just as coupled with this commit. But I don't mind it, I guess, since it should make further decoupling easier (since you can handle each case separately, vs. an unknown number of cases through a Display/Debug impl). |
(iirc there were rather few dependencies on pprust left in libsyntax after #65324 so in a larger context this should bring us closer to that goal.) |
@bors r+ Changes look correct to me, and it seems to be passing CI. Some of the debug output is probably technically worse now but not in a way that seems immediately bad to me so we can probably leave it be for the time being. |
📌 Commit ab8105e has been approved by |
Remove implicit dependencies on syntax::pprust Part of rust-lang#65324. The main goal here is to facilitate the eventual move of pprust out from libsyntax and because an AST definition typically should not depend on its pretty printer. r? @estebank
Remove implicit dependencies on syntax::pprust Part of rust-lang#65324. The main goal here is to facilitate the eventual move of pprust out from libsyntax and because an AST definition typically should not depend on its pretty printer. r? @estebank
Rollup of 7 pull requests Successful merges: - #65215 (Add long error explanation for E0697) - #65292 (Print lifetimes with backticks) - #65362 (syntax: consolidate function parsing in item.rs) - #65363 (Remove implicit dependencies on syntax::pprust) - #65379 (refactor session::config::build_session_options_and_crate_config) - #65392 (Move `Nonterminal::to_tokenstream` to parser & don't rely directly on parser in lowering) - #65395 (Add some tests for fixed ICEs) Failed merges: r? @ghost
Rollup of 7 pull requests Successful merges: - #65215 (Add long error explanation for E0697) - #65292 (Print lifetimes with backticks) - #65362 (syntax: consolidate function parsing in item.rs) - #65363 (Remove implicit dependencies on syntax::pprust) - #65379 (refactor session::config::build_session_options_and_crate_config) - #65392 (Move `Nonterminal::to_tokenstream` to parser & don't rely directly on parser in lowering) - #65395 (Add some tests for fixed ICEs) Failed merges: r? @ghost
implicit pprust dependencies were removed and now require explicit usage rust-lang/rust#65363
Part of #65324.
The main goal here is to facilitate the eventual move of pprust out from libsyntax and because an AST definition typically should not depend on its pretty printer.
r? @estebank