Do not gather local all together at the beginning of typeck

Author: compiler-errors

compiler/rustc_hir_typeck/src/_match.rs

@@ -11,7 +11,7 @@ use rustc_trait_selection::traits::{
 use tracing::{debug, instrument};
 
 use crate::coercion::{AsCoercionSite, CoerceMany};
-use crate::{Diverges, Expectation, FnCtxt, Needs};
+use crate::{Diverges, Expectation, FnCtxt, GatherLocalsVisitor, Needs};
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     #[instrument(skip(self), level = "debug", ret)]
@@ -43,6 +43,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // #55810: Type check patterns first so we get types for all bindings.
         let scrut_span = scrut.span.find_ancestor_inside(expr.span).unwrap_or(scrut.span);
         for arm in arms {
+            GatherLocalsVisitor::gather_from_arm(self, arm);
+
             self.check_pat_top(arm.pat, scrutinee_ty, Some(scrut_span), Some(scrut), None);
         }
 

compiler/rustc_hir_typeck/src/check.rs

@@ -3,7 +3,6 @@ use std::cell::RefCell;
 use rustc_abi::ExternAbi;
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
-use rustc_hir::intravisit::Visitor;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir_analysis::check::check_function_signature;
 use rustc_infer::infer::RegionVariableOrigin;
@@ -50,7 +49,9 @@ pub(super) fn check_fn<'a, 'tcx>(
 
     let span = body.value.span;
 
-    GatherLocalsVisitor::new(fcx).visit_body(body);
+    for param in body.params {
+        GatherLocalsVisitor::gather_from_param(fcx, param);
+    }
 
     // C-variadic fns also have a `VaList` input that's not listed in `fn_sig`
     // (as it's created inside the body itself, not passed in from outside).

compiler/rustc_hir_typeck/src/expr.rs

@@ -16,7 +16,6 @@ use rustc_errors::{
 };
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::DefId;
-use rustc_hir::intravisit::Visitor;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{ExprKind, HirId, QPath};
 use rustc_hir_analysis::NoVariantNamed;
@@ -50,8 +49,8 @@ use crate::errors::{
     YieldExprOutsideOfCoroutine,
 };
 use crate::{
-    BreakableCtxt, CoroutineTypes, Diverges, FnCtxt, Needs, TupleArgumentsFlag, cast,
-    fatally_break_rust, report_unexpected_variant_res, type_error_struct,
+    BreakableCtxt, CoroutineTypes, Diverges, FnCtxt, GatherLocalsVisitor, Needs,
+    TupleArgumentsFlag, cast, fatally_break_rust, report_unexpected_variant_res, type_error_struct,
 };
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
@@ -1518,11 +1517,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let_expr: &'tcx hir::LetExpr<'tcx>,
         hir_id: HirId,
     ) -> Ty<'tcx> {
+        GatherLocalsVisitor::gather_from_let_expr(self, let_expr, hir_id);
+
         // for let statements, this is done in check_stmt
         let init = let_expr.init;
         self.warn_if_unreachable(init.hir_id, init.span, "block in `let` expression");
+
         // otherwise check exactly as a let statement
         self.check_decl((let_expr, hir_id).into());
+
         // but return a bool, for this is a boolean expression
         if let ast::Recovered::Yes(error_guaranteed) = let_expr.recovered {
             self.set_tainted_by_errors(error_guaranteed);
@@ -1827,7 +1830,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // Create a new function context.
         let def_id = block.def_id;
         let fcx = FnCtxt::new(self, self.param_env, def_id);
-        crate::GatherLocalsVisitor::new(&fcx).visit_body(body);
 
         let ty = fcx.check_expr_with_expectation(body.value, expected);
         fcx.require_type_is_sized(ty, body.value.span, ObligationCauseCode::SizedConstOrStatic);

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -37,8 +37,8 @@ use crate::method::probe::IsSuggestion;
 use crate::method::probe::Mode::MethodCall;
 use crate::method::probe::ProbeScope::TraitsInScope;
 use crate::{
-    BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy, Needs, TupleArgumentsFlag, errors,
-    struct_span_code_err,
+    BreakableCtxt, Diverges, Expectation, FnCtxt, GatherLocalsVisitor, LoweredTy, Needs,
+    TupleArgumentsFlag, errors, struct_span_code_err,
 };
 
 rustc_index::newtype_index! {
@@ -1765,6 +1765,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
     /// Type check a `let` statement.
     fn check_decl_local(&self, local: &'tcx hir::LetStmt<'tcx>) {
+        GatherLocalsVisitor::gather_from_local(self, local);
+
         let ty = self.check_decl(local.into());
         self.write_ty(local.hir_id, ty);
         if local.pat.is_never_pattern() {

compiler/rustc_hir_typeck/src/gather_locals.rs

@@ -55,6 +55,14 @@ impl<'a> From<(&'a hir::LetExpr<'a>, HirId)> for Declaration<'a> {
     }
 }
 
+/// The `GatherLocalsVisitor` is responsible for initializing local variable types
+/// in the [`ty::TypeckResults`] for all subpatterns in statements and expressions
+/// like `let`, `match`, and params of function bodies. It also adds `Sized` bounds
+/// for these types (with exceptions for unsized feature gates like `unsized_fn_params`).
+///
+/// Failure to visit locals will cause an ICE in writeback when the local's type is
+/// resolved. Visiting locals twice will ICE in the `GatherLocalsVisitor`, since it
+/// will overwrite the type previously stored in the local.
 pub(super) struct GatherLocalsVisitor<'a, 'tcx> {
     fcx: &'a FnCtxt<'a, 'tcx>,
     // parameters are special cases of patterns, but we want to handle them as
@@ -63,22 +71,50 @@ pub(super) struct GatherLocalsVisitor<'a, 'tcx> {
     outermost_fn_param_pat: Option<(Span, HirId)>,
 }
 
+// N.B. additional `gather_*` functions should be careful to only walk the pattern
+// for new expressions, since visiting sub-expressions or nested bodies may initialize
+// locals which are not conceptually owned by the gathered statement or expression.
 impl<'a, 'tcx> GatherLocalsVisitor<'a, 'tcx> {
-    pub(super) fn new(fcx: &'a FnCtxt<'a, 'tcx>) -> Self {
-        Self { fcx, outermost_fn_param_pat: None }
+    pub(crate) fn gather_from_local(fcx: &'a FnCtxt<'a, 'tcx>, local: &'tcx hir::LetStmt<'tcx>) {
+        let mut visitor = GatherLocalsVisitor { fcx, outermost_fn_param_pat: None };
+        visitor.declare(local.into());
+        visitor.visit_pat(local.pat);
+    }
+
+    pub(crate) fn gather_from_let_expr(
+        fcx: &'a FnCtxt<'a, 'tcx>,
+        let_expr: &'tcx hir::LetExpr<'tcx>,
+        expr_hir_id: hir::HirId,
+    ) {
+        let mut visitor = GatherLocalsVisitor { fcx, outermost_fn_param_pat: None };
+        visitor.declare((let_expr, expr_hir_id).into());
+        visitor.visit_pat(let_expr.pat);
+    }
+
+    pub(crate) fn gather_from_param(fcx: &'a FnCtxt<'a, 'tcx>, param: &'tcx hir::Param<'tcx>) {
+        let mut visitor = GatherLocalsVisitor {
+            fcx,
+            outermost_fn_param_pat: Some((param.ty_span, param.hir_id)),
+        };
+        visitor.visit_pat(param.pat);
+    }
+
+    pub(crate) fn gather_from_arm(fcx: &'a FnCtxt<'a, 'tcx>, local: &'tcx hir::Arm<'tcx>) {
+        let mut visitor = GatherLocalsVisitor { fcx, outermost_fn_param_pat: None };
+        visitor.visit_pat(local.pat);
     }
 
     fn assign(&mut self, span: Span, nid: HirId, ty_opt: Option<Ty<'tcx>>) -> Ty<'tcx> {
         match ty_opt {
             None => {
                 // Infer the variable's type.
                 let var_ty = self.fcx.next_ty_var(span);
-                self.fcx.locals.borrow_mut().insert(nid, var_ty);
+                assert_eq!(self.fcx.locals.borrow_mut().insert(nid, var_ty), None);
                 var_ty
             }
             Some(typ) => {
                 // Take type that the user specified.
-                self.fcx.locals.borrow_mut().insert(nid, typ);
+                assert_eq!(self.fcx.locals.borrow_mut().insert(nid, typ), None);
                 typ
             }
         }
@@ -133,13 +169,6 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
         intravisit::walk_expr(self, expr)
     }
 
-    fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
-        let old_outermost_fn_param_pat =
-            self.outermost_fn_param_pat.replace((param.ty_span, param.hir_id));
-        intravisit::walk_param(self, param);
-        self.outermost_fn_param_pat = old_outermost_fn_param_pat;
-    }
-
     // Add pattern bindings.
     fn visit_pat(&mut self, p: &'tcx hir::Pat<'tcx>) {
         if let PatKind::Binding(_, _, ident, _) = p.kind {

compiler/rustc_hir_typeck/src/lib.rs

@@ -46,7 +46,6 @@ use rustc_errors::codes::*;
 use rustc_errors::{Applicability, ErrorGuaranteed, pluralize, struct_span_code_err};
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::intravisit::Visitor;
 use rustc_hir::{HirId, HirIdMap, Node};
 use rustc_hir_analysis::check::check_abi;
 use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
@@ -191,9 +190,6 @@ fn typeck_with_inspect<'tcx>(
         let wf_code = ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(def_id)));
         fcx.register_wf_obligation(expected_type.into(), body.value.span, wf_code);
 
-        // Gather locals in statics (because of block expressions).
-        GatherLocalsVisitor::new(&fcx).visit_body(body);
-
         fcx.check_expr_coercible_to_type(body.value, expected_type, None);
 
         fcx.write_ty(id, expected_type);

tests/ui/async-await/async-closures/def-path.stderr

@@ -5,11 +5,11 @@ LL |     let x = async || {};
    |                      -- the expected `async` closure body
 LL |
 LL |     let () = x();
-   |         ^^   --- this expression has type `{static main::{closure#0}::{closure#0}<?17t> upvar_tys=?16t resume_ty=ResumeTy yield_ty=() return_ty=() witness=?6t}`
+   |         ^^   --- this expression has type `{static main::{closure#0}::{closure#0}<?17t> upvar_tys=?16t resume_ty=ResumeTy yield_ty=() return_ty=() witness=?5t}`
    |         |
    |         expected `async` closure body, found `()`
    |
-   = note: expected `async` closure body `{static main::{closure#0}::{closure#0}<?17t> upvar_tys=?16t resume_ty=ResumeTy yield_ty=() return_ty=() witness=?6t}`
+   = note: expected `async` closure body `{static main::{closure#0}::{closure#0}<?17t> upvar_tys=?16t resume_ty=ResumeTy yield_ty=() return_ty=() witness=?5t}`
                          found unit type `()`
 
 error: aborting due to 1 previous error

tests/ui/closures/print/closure-print-generic-trim-off-verbose-2.stderr

@@ -9,7 +9,7 @@ LL |         let c1 : () = c;
    |                  expected due to this
    |
    = note: expected unit type `()`
-                found closure `{mod1::f<T>::{closure#0} closure_kind_ty=?8t closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=?7t}`
+                found closure `{mod1::f<T>::{closure#0} closure_kind_ty=?7t closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=?6t}`
 help: use parentheses to call this closure
    |
 LL |         let c1 : () = c();

tests/ui/closures/print/closure-print-generic-verbose-2.stderr

@@ -9,7 +9,7 @@ LL |         let c1 : () = c;
    |                  expected due to this
    |
    = note: expected unit type `()`
-                found closure `{f<T>::{closure#0} closure_kind_ty=?8t closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=?7t}`
+                found closure `{f<T>::{closure#0} closure_kind_ty=?7t closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=?6t}`
 help: use parentheses to call this closure
    |
 LL |         let c1 : () = c();

tests/ui/const-generics/const-arg-in-const-arg.min.stderr

@@ -240,41 +240,6 @@ note: the late bound lifetime parameter is introduced here
 LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
    |              ^^
 
-error[E0747]: unresolved item provided when a constant was expected
-  --> $DIR/const-arg-in-const-arg.rs:36:24
-   |
-LL |     let _: Foo<{ bar::<N>() }>;
-   |                        ^
-   |
-help: if this generic argument was intended as a const parameter, surround it with braces
-   |
-LL |     let _: Foo<{ bar::<{ N }>() }>;
-   |                        +   +
-
-error[E0794]: cannot specify lifetime arguments explicitly if late bound lifetime parameters are present
-  --> $DIR/const-arg-in-const-arg.rs:38:24
-   |
-LL |     let _: Foo<{ faz::<'a>(&()) }>;
-   |                        ^^
-   |
-note: the late bound lifetime parameter is introduced here
-  --> $DIR/const-arg-in-const-arg.rs:10:14
-   |
-LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
-   |              ^^
-
-error[E0794]: cannot specify lifetime arguments explicitly if late bound lifetime parameters are present
-  --> $DIR/const-arg-in-const-arg.rs:41:24
-   |
-LL |     let _: Foo<{ faz::<'b>(&()) }>;
-   |                        ^^
-   |
-note: the late bound lifetime parameter is introduced here
-  --> $DIR/const-arg-in-const-arg.rs:10:14
-   |
-LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
-   |              ^^
-
 error: constant expression depends on a generic parameter
   --> $DIR/const-arg-in-const-arg.rs:25:17
    |
@@ -326,6 +291,41 @@ note: the late bound lifetime parameter is introduced here
 LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
    |              ^^
 
+error[E0747]: unresolved item provided when a constant was expected
+  --> $DIR/const-arg-in-const-arg.rs:36:24
+   |
+LL |     let _: Foo<{ bar::<N>() }>;
+   |                        ^
+   |
+help: if this generic argument was intended as a const parameter, surround it with braces
+   |
+LL |     let _: Foo<{ bar::<{ N }>() }>;
+   |                        +   +
+
+error[E0794]: cannot specify lifetime arguments explicitly if late bound lifetime parameters are present
+  --> $DIR/const-arg-in-const-arg.rs:38:24
+   |
+LL |     let _: Foo<{ faz::<'a>(&()) }>;
+   |                        ^^
+   |
+note: the late bound lifetime parameter is introduced here
+  --> $DIR/const-arg-in-const-arg.rs:10:14
+   |
+LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
+   |              ^^
+
+error[E0794]: cannot specify lifetime arguments explicitly if late bound lifetime parameters are present
+  --> $DIR/const-arg-in-const-arg.rs:41:24
+   |
+LL |     let _: Foo<{ faz::<'b>(&()) }>;
+   |                        ^^
+   |
+note: the late bound lifetime parameter is introduced here
+  --> $DIR/const-arg-in-const-arg.rs:10:14
+   |
+LL | const fn faz<'a>(_: &'a ()) -> usize { 13 }
+   |              ^^
+
 error[E0747]: unresolved item provided when a constant was expected
   --> $DIR/const-arg-in-const-arg.rs:45:27
    |

tests/ui/const-generics/generic_const_exprs/dependence_lint.gce.stderr

@@ -1,38 +1,38 @@
-error: overly complex generic constant
-  --> $DIR/dependence_lint.rs:21:17
-   |
-LL |     let _: [u8; if true { size_of::<T>() } else { 3 }]; // error on stable, error with gce
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ control flow is not supported in generic constants
-   |
-   = help: consider moving this anonymous constant into a `const` function
-
 error: unconstrained generic constant
-  --> $DIR/dependence_lint.rs:14:12
+  --> $DIR/dependence_lint.rs:10:9
    |
-LL |     let _: [u8; size_of::<*mut T>()]; // error on stable, error with gce
-   |            ^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     [0; size_of::<*mut T>()]; // lint on stable, error with `generic_const_exprs`
+   |         ^^^^^^^^^^^^^^^^^^^
    |
 help: try adding a `where` bound
    |
 LL | fn foo<T>() where [(); size_of::<*mut T>()]: {
    |             ++++++++++++++++++++++++++++++++
 
+error: overly complex generic constant
+  --> $DIR/dependence_lint.rs:17:9
+   |
+LL |     [0; if false { size_of::<T>() } else { 3 }]; // lint on stable, error with gce
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ control flow is not supported in generic constants
+   |
+   = help: consider moving this anonymous constant into a `const` function
+
 error: unconstrained generic constant
-  --> $DIR/dependence_lint.rs:10:9
+  --> $DIR/dependence_lint.rs:14:12
    |
-LL |     [0; size_of::<*mut T>()]; // lint on stable, error with `generic_const_exprs`
-   |         ^^^^^^^^^^^^^^^^^^^
+LL |     let _: [u8; size_of::<*mut T>()]; // error on stable, error with gce
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 help: try adding a `where` bound
    |
 LL | fn foo<T>() where [(); size_of::<*mut T>()]: {
    |             ++++++++++++++++++++++++++++++++
 
 error: overly complex generic constant
-  --> $DIR/dependence_lint.rs:17:9
+  --> $DIR/dependence_lint.rs:21:17
    |
-LL |     [0; if false { size_of::<T>() } else { 3 }]; // lint on stable, error with gce
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ control flow is not supported in generic constants
+LL |     let _: [u8; if true { size_of::<T>() } else { 3 }]; // error on stable, error with gce
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ control flow is not supported in generic constants
    |
    = help: consider moving this anonymous constant into a `const` function