-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add support for parsing with rust-analyzer instead of librustc_parse #70761
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
e677922
to
f8f8ade
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This adds a new librustc_parse_ra library that implements the same interface of librustc_parse, but uses rust-analyzer to parse the code, and then converts the rust-analyzer AST to the rustc AST, and adds a new -Z parse-with-rust-analyzer option that makes rustc use it, gated by a new rust_analyzer feature flag and config.toml option. The code is preliminary but should be essentially complete, and capable of building large projects that only use the Rust 2018 edition (Rust 2015 is not currently supported by rust-analyzer). Note that this is likely to accept invalid code, is not fully complete, and provides worse error messages than the current parser, so it cannot be stabilized without significant additional work.
f8f8ade
to
3f40227
Compare
This can be done in Cargo.toml (manual): ra_syntax = { git = "https://github.com/luca-barbieri/rust-analyzer", branch = "rustc" } I'm not sure whether tidy will like it though. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
r? @Centril or @petrochenkov |
I've looked at it, and, first of all, I am very impressed! I've compiled some far from trivial crates locally with rust-analyzer parser enabled, and ... it just works ? This is a huge milestone for me personally, because I always fear that I am just doing some "fake IDE stuff", which is definitely not a real compiler. This proves that my fears are unfounded :-) That said, I at the moment see this PR as experimentation and a proof of concept. I am personally not ready to bet that this approach is what we should shipl (but I also don't think that this is something we definitely should not ship: I just don't know). What follows is my summary of the PR. The approach taken here is lowering rust-analyzer's syntax tree data structure to rustc's ast. That is, this PR depends both on "pure parsing" side of rust-analyzer (ra_parser), as well as on the specific concrete syntax tree implementation (ra_syntax). This is different from the approach I primarily had in mind during the design meetings, where we share only the parser itself. It is impressive and reassuring that this implementation is purely additive, and is only 3k lines in rustc. The main hidden gotcha here is that it works only for "top-level" files, macros are still parsed with the rustc parser. It is doesn't seem a fundamental limitation to me, we can extend this approach to deal with macros as well. I see two primary drawbacks of this approach. First, it adds dependency on the syntax tree data structure. While I think that we can make the parsing API relatively stable (as the essence of parsing is simple: inserted a nonterminal-marked parenthesis into the input stream), the syntax tree will probably be much more volatile. In rust-analyzer, we've rewritten the tree a handful of times already, and there's at least one more rewrite in the works. Second, it is guaranteed to perform worse than what we have today in rustc, because we allocate another intermediate data structure. It would be interesting to get some preliminary data about how costly is this in practice, since we already have the code to do that. The benchmarks should be taken with a tablespoon-full of salt, however, as rust-analyzer's parser wasn't particularly optimized or even systematically profiled. However, this approach also has a couple of similarly-sized benefits. First, it seems much simpler than extracting event-based parsing. The code for cst->ast mapping is simple, understandable and not that long. Second, it is close to the end-state we want to see. I do think that rustc's front-end should use concrete syntax tree, both because macros are defined in terms of concrete syntax, and because many IDE features are much easier to implement in terms of concrete syntax. So, long term I'd love to see rustc's ast dissolved into CST (for the purposes of macro expansion and initial name resolution), non-tree of id-based items, and arena-allocated compact lowerd expression bodies. I think the immediate action-items here are, for @rust-lang/compiler, to familiarize themselves with this work and, for @matklad, to merge the rust-analyzer side of this work (as it is very useful regardless, and something I wanted to see done for ages). |
r? @eddyb |
Nominating for @rust-lang/compiler discussion, but I suspect this requires a MCP. |
Discussed in today's T-compiler weekly meeting. This change is significant; it needs to go through the "major change proposal" (MCP) process (the documentation for which is still in flight, but you can read about it over on the MCP RFC marking as blocked-on-author so that they know that they need to file an MCP to make progress here.
|
☔ The latest upstream changes (presumably #71007) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from Triage: Hi @luca-barbieri - quick ping to fix the conflicts when you get a chance. |
Closing due to inactivity, feel free to reopen or create another PR when you're ready to continue (but maybe you want to create the major change proposal first i guess). |
This adds a new librustc_parse_ra library that implements the same interface of librustc_parse, but uses rust-analyzer to parse the code, and then converts the rust-analyzer AST to the rustc AST, and adds a new -Z parse-with-rust-analyzer option that makes rustc use it, gated by a new rust_analyzer feature flag and config.toml option.
The code is preliminary but should be essentially complete, and capable of building large projects that only use the Rust 2018 edition (Rust 2015 is not currently supported by rust-analyzer).
This is likely to accept invalid code, is not fully complete, and provides worse error messages than the current parser, so it cannot be stabilized without significant additional work, and is currently only useful for experimental and development purposes.
It depends on a slightly modified version of rust-analyzer from rust-lang/rust-analyzer#3838 - you will need to make sure that the "ra_syntax" dependency comes from that source