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

[Veykril]

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]

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]

parameter type handling is here

rust-analyzer/crates/hir-ty/src/infer.rs

Lines 864 to 885 in 3ae93bc

	let mut param_tys = param_tys.into_iter().chain(iter::repeat(self.table.new_type_var()));
	if let Some(self_param) = self.body.self_param {
	if let Some(ty) = param_tys.next() {
	let ty = self.insert_type_vars(ty);
	let ty = self.normalize_associated_types_in(ty);
	self.write_binding_ty(self_param, ty);
	}
	}
	let mut tait_candidates = FxHashSet::default();
	for (ty, pat) in param_tys.zip(&*self.body.params) {
	let ty = self.insert_type_vars(ty);
	let ty = self.normalize_associated_types_in(ty);
	self.infer_top_pat(*pat, &ty);
	if ty
	.data(Interner)
	.flags
	.intersects(TypeFlags::HAS_TY_OPAQUE.union(TypeFlags::HAS_TY_INFER))
	{
	tait_candidates.insert(ty);
	}
	}

The error flag setting (which is supposed to prevent mir building) is done here

rust-analyzer/crates/hir-ty/src/infer.rs

Line 676 in 3ae93bc

pub(crate) fn resolve_all(self) -> InferenceResult {

[davidbarsky]

I think it's very easy to get this panic inside of https://github.com/oxidecomputer/omicron due to its Diesel usage. Lukas said:

i wonder if we should just kick that out, its clearly a reachable branch at this point x) (we need to restructure how we approach mir lowering to fix this I believe)