Parser updates for #67131

[lnicola]

See rust-lang/rust#67131.

I tried the following as a quick test:

#[cfg(FALSE)]
trait T {
    default fn foo(&self);
}

struct S;

#[cfg(FALSE)]
impl S {
    const C: i32;
    type Ty;
    type Qux: Debug;

    fn foo();
    fn bar(..., x: usize) {}
    fn baz(...) {}
}

fn main() {}

RA reports parsing errors on default fn in a trait and on fn bar(..., x: usize).

[lnicola]

Well, that PR has extensive tests. I wonder if we can/should import the rustc parser tests...

[Centril]

Well, that PR has extensive tests. I wonder if we can/should import the rustc parser tests...

You might want to consider something like the approach syn uses, cc @dtolnay

[matklad]

yeah...

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_lexer into a separate repo/directory/workspace as a self-sufficient isolated crate.

[Centril]

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 //~ ERROR annotations in the .rs files. It's both easier to review such annotations & UI tests, and its harder to miss in code reviews that annotations have been changed.

[matklad]

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.

[Centril]

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_parser has really improved as the later bits depending on it have been reduced substantially. For example, librustc and librustc_typeck do 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/ui that 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" }

[lnicola]

Update: we support default, but not ... in the middle of a parameter list.