Skip to content

Parser updates for #67131 #3578

Description

@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).

Activity

  1. lnicola commented on Mar 13, 2020

    @lnicola
    MemberAuthor

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

  2. Centril commented on Mar 23, 2020

    @Centril
    Contributor

    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

  3. matklad commented on Mar 23, 2020

    @matklad
    Contributor

    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.

  4. Centril commented on Mar 24, 2020

    @Centril
    Contributor

    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.

  5. matklad commented on Mar 24, 2020

    @matklad
    Contributor

    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.

  6. Centril commented on Mar 24, 2020

    @Centril
    Contributor

    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" }
    
  7. added
    S-actionableSomeone could pick this issue up and work on it right now
    on Jan 22, 2021
  8. lnicola commented on Jun 30, 2021

    @lnicola
    MemberAuthor

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

  9. added a commit that references this issue on Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-parserparser issuesS-actionableSomeone could pick this issue up and work on it right now

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions