Repository navigation
Parser updates for #67131 #3578
Description
Activity
Well, that PR has extensive tests. I wonder if we can/should import the
rustcparser tests...Well, that PR has extensive tests. I wonder if we can/should import the
rustcparser tests...You might want to consider something like the approach
synuses, cc @dtolnayyeah...
On the one hand, we should prioritize merging with rustc parser as soon as possible.
On the other hand, I actually think that ra's parser tests, which just dump lossless parse tree, are long-term a better solution than the fully integrated rustc tests, so it might make sense to continue investing in our testing infra. Or, more specifically, that rustc should ultimately be tested by UI tests, but, for the general development, there also should be "layer" tests, which, eg, exercise only the parser, without touching/building the rest of the compiler.
This is what I didn't like about rustc_lexer extraction -- this bit of code is still tested mostly via ui tests, so you can't pull
rustc_lexerinto a separate repo/directory/workspace as a self-sufficient isolated crate.On the other hand, I actually think that ra's parser tests, which just dump lossless parse tree, are long-term a better solution than the fully integrated rustc tests, so it might make sense to continue investing in our testing infra. Or, more specifically, that rustc should ultimately be tested by UI tests, but, for the general development, there also should be "layer" tests, which, eg, exercise only the parser, without touching/building the rest of the compiler.
I think that's fine as long the UI tests are there exercising important logic in the parser. I don't see dumping lossless parse trees as something more robust however as compared to
//~ ERRORannotations in the.rsfiles. It's both easier to review such annotations & UI tests, and its harder to miss in code reviews that annotations have been changed.Hm, I agree with your reasoning. I think I confuse two different differences here:
- the first is output format, serialized tree vs error messages. I do think that error messages are actually a better approach
- the second is the "level of integration": ra parser tests build only the parser, rustc parser tests build the whole compiler, and it is the "it is not fun to wait for rustc_typecheck to compile" I want to get rid of.
But, in theory, it should be possible to produce UI tests from the parser library, without the rest of the compiler.
the second is the "level of integration": ra parser tests build only the parser, rustc parser tests build the whole compiler, and it is the "it is not fun to wait for rustc_typecheck to compile" I want to get rid of.
For what its worth, once you have built the compiler then incremental compilation for modifying
rustc_parserhas really improved as the later bits depending on it have been reduced substantially. For example,librustcandlibrustc_typeckdo not depend on it and don't have to be recompiled. It's mostly the first compilation with./x.py test --stage 1 --pass check --bless src/test/uithat takes more time when starting on a new branch, but subsequent./x.py --keep-stage 1 test --stage 1 --pass check --bless src/test/uis are quick.I have ideas to improve this even further (as I don't want to wait a lot when hacking on the parser myself), but the biggest wins are already taken:
centril@centrilnas2:~/programming/rust/rust-gamma$ rg librustc_parse src/librustc_driver/Cargo.toml 26:rustc_parse = { path = "../librustc_parse" } src/librustc_builtin_macros/Cargo.toml 20:rustc_parse = { path = "../librustc_parse" } src/librustc_ast_passes/Cargo.toml 18:rustc_parse = { path = "../librustc_parse" } src/librustc_save_analysis/Cargo.toml 18:rustc_parse = { path = "../librustc_parse" } src/librustc_expand/Cargo.toml 24:rustc_parse = { path = "../librustc_parse" } src/librustc_interface/Cargo.toml 20:rustc_parse = { path = "../librustc_parse" }- addedA-parserparser issuesparser issuesS-actionableSomeone could pick this issue up and work on it right nowSomeone could pick this issue up and work on it right now
on Jan 22, 2021 Update: we support
default, but not...in the middle of a parameter list.
See rust-lang/rust#67131.
I tried the following as a quick test:
RA reports parsing errors on
default fnin a trait and onfn bar(..., x: usize).