rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.33k stars 1.61k forks source link

`fn f((arg: (usize, bool)) {}` triggers `Only tuple has tuple field` error #18339

Open Veykril opened 1 month ago

Veykril commented 1 month ago

I've found a more minimal reproduction. Going from fn f(arg: (usize, bool)) {} to fn f((arg: (usize, bool)) {} is enough to crash rust-analyzer with "internal error: entered unreachable code: Only tuple has tuple field".

Originally posted by @davidbarsky in #15090

The parse tree for fn f((arg: (usize, bool)) {} is

FN@0..28
  FN_KW@0..2 "fn"
  WHITESPACE@2..3 " "
  NAME@3..4
    IDENT@3..4 "f"
  PARAM_LIST@4..25
    L_PAREN@4..5 "("
    PARAM@5..25
      TUPLE_PAT@5..25
        L_PAREN@5..6 "("
        IDENT_PAT@6..9
          NAME@6..9
            IDENT@6..9 "arg"
        ERROR@9..10
          COLON@9..10 ":"
        WHITESPACE@10..11 " "
        TUPLE_PAT@11..24
          L_PAREN@11..12 "("
          IDENT_PAT@12..17
            NAME@12..17
              IDENT@12..17 "usize"
          COMMA@17..18 ","
          WHITESPACE@18..19 " "
          IDENT_PAT@19..23
            NAME@19..23
              IDENT@19..23 "bool"
          R_PAREN@23..24 ")"
        R_PAREN@24..25 ")"
  WHITESPACE@25..26 " "
  BLOCK_EXPR@26..28
    STMT_LIST@26..28
      L_CURLY@26..27 "{"
      R_CURLY@27..28 "}"

so its basically parses as a function with a tuple pattern containing an ident pattern and another tuple pattern, missing the closing paren for the parameter list.

The HIR is

fn f((arg, (usize, bool)): {unknown}) -> () {}

which is expected. The mir is

fn f() {
    let _0: ();
    let _1: {unknown};
    let arg_2: {unknown};
    let usize_3: {unknown};
    let bool_4: {unknown};

    'bb0: {
        StorageLive(arg_2)
        arg_2 = _1.0;
        StorageLive(usize_3)
        usize_3 = _1.1.0;
        StorageLive(bool_4)
        bool_4 = _1.1.1;
        Terminator { span: ExprId(Idx::<Expr>(0)), kind: Drop { place: Place { local: Idx::<Local>(4), projection: ProjectionId(0) }, target: Idx::<BasicBlock>(1), unwind: None } };
    }

    'bb1: {
        StorageDead(bool_4)
        Terminator { span: ExprId(Idx::<Expr>(0)), kind: Drop { place: Place { local: Idx::<Local>(3), projection: ProjectionId(0) }, target: Idx::<BasicBlock>(2), unwind: None } };
    }

    'bb2: {
        StorageDead(usize_3)
        Terminator { span: ExprId(Idx::<Expr>(0)), kind: Drop { place: Place { local: Idx::<Local>(2), projection: ProjectionId(0) }, target: Idx::<BasicBlock>(3), unwind: None } };
    }

    'bb3: {
        StorageDead(arg_2)
        Terminator { span: ExprId(Idx::<Expr>(0)), kind: Drop { place: Place { local: Idx::<Local>(1), projection: ProjectionId(0) }, target: Idx::<BasicBlock>(4), unwind: None } };
    }

    'bb4: {
        StorageDead(_1)
        Terminator { span: ExprId(Idx::<Expr>(0)), kind: Return };
    }
}

which is obviously bad, we shouldn't have a mir with unknowns in it in the first place!

And hence, as it turns out, just a fn f((a,)) {} also reproduces this (likely any non ident pattern in a param with a missing type. We probably just forget to check the function param pattern types if they contain unknowns and marking the type check result as tainted appropriately.

davidbarsky commented 1 month ago

marking the type check result as tainted appropriately.

I've been meaning to better understand hir in ra—do you mind pointing me to where I can try implementing a fix?

Veykril commented 1 month ago

parameter type handling is here https://github.com/rust-lang/rust-analyzer/blob/3ae93bcb82b2e34754fa96d05dbcecb6de3ede0e/crates/hir-ty/src/infer.rs#L864-L885

The error flag setting (which is supposed to prevent mir building) is done here https://github.com/rust-lang/rust-analyzer/blob/3ae93bcb82b2e34754fa96d05dbcecb6de3ede0e/crates/hir-ty/src/infer.rs#L676