Rollup merge of rust-lang#137045 - BoxyUwU:defer_repeat_expr_checks, r=compiler-errors

Defer repeat expr `Copy` checks to end of type checking

Fixes rust-lang#110443

Defers repeat expr checks that the element type is `Copy` when the length is > 1 (or generic) to end of typeck so that under `generic_arg_infer` repeat exprs are able to have an inferred count, e.g. `let a: [_; 1] = [String::new(); _];`.

Currently the deferring is gated under `generic_arg_infer` though I intend to separately types FCP deferring the checks even outside of `generic_arg_infer` if we wind up not going with an alternative.

Author: matthiaskrgr

compiler/rustc_hir_typeck/src/expr.rs

@@ -1853,12 +1853,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return Ty::new_error(tcx, guar);
         }
 
+        // We defer checking whether the element type is `Copy` as it is possible to have
+        // an inference variable as a repeat count and it seems unlikely that `Copy` would
+        // have inference side effects required for type checking to succeed.
+        if tcx.features().generic_arg_infer() {
+            self.deferred_repeat_expr_checks.borrow_mut().push((element, element_ty, count));
         // If the length is 0, we don't create any elements, so we don't copy any.
         // If the length is 1, we don't copy that one element, we move it. Only check
         // for `Copy` if the length is larger, or unevaluated.
-        // FIXME(min_const_generic_exprs): We could perhaps defer this check so that
-        // we don't require `<?0t as Tr>::CONST` doesn't unnecessarily require `Copy`.
-        if count.try_to_target_usize(tcx).is_none_or(|x| x > 1) {
+        } else if count.try_to_target_usize(self.tcx).is_none_or(|x| x > 1) {
             self.enforce_repeat_element_needs_copy_bound(element, element_ty);
         }
 
@@ -1868,7 +1871,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     }
 
     /// Requires that `element_ty` is `Copy` (unless it's a const expression itself).
-    fn enforce_repeat_element_needs_copy_bound(
+    pub(super) fn enforce_repeat_element_needs_copy_bound(
         &self,
         element: &hir::Expr<'_>,
         element_ty: Ty<'tcx>,

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -85,33 +85,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         })
     }
 
-    /// Resolves type and const variables in `ty` if possible. Unlike the infcx
+    /// Resolves type and const variables in `t` if possible. Unlike the infcx
     /// version (resolve_vars_if_possible), this version will
     /// also select obligations if it seems useful, in an effort
     /// to get more type information.
     // FIXME(-Znext-solver): A lot of the calls to this method should
     // probably be `try_structurally_resolve_type` or `structurally_resolve_type` instead.
     #[instrument(skip(self), level = "debug", ret)]
-    pub(crate) fn resolve_vars_with_obligations(&self, mut ty: Ty<'tcx>) -> Ty<'tcx> {
+    pub(crate) fn resolve_vars_with_obligations<T: TypeFoldable<TyCtxt<'tcx>>>(
+        &self,
+        mut t: T,
+    ) -> T {
         // No Infer()? Nothing needs doing.
-        if !ty.has_non_region_infer() {
+        if !t.has_non_region_infer() {
             debug!("no inference var, nothing needs doing");
-            return ty;
+            return t;
         }
 
-        // If `ty` is a type variable, see whether we already know what it is.
-        ty = self.resolve_vars_if_possible(ty);
-        if !ty.has_non_region_infer() {
-            debug!(?ty);
-            return ty;
+        // If `t` is a type variable, see whether we already know what it is.
+        t = self.resolve_vars_if_possible(t);
+        if !t.has_non_region_infer() {
+            debug!(?t);
+            return t;
         }
 
         // If not, try resolving pending obligations as much as
         // possible. This can help substantially when there are
         // indirect dependencies that don't seem worth tracking
         // precisely.
         self.select_obligations_where_possible(|_| {});
-        self.resolve_vars_if_possible(ty)
+        self.resolve_vars_if_possible(t)
     }
 
     pub(crate) fn record_deferred_call_resolution(
@@ -1454,7 +1457,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         sp: Span,
         ct: ty::Const<'tcx>,
     ) -> ty::Const<'tcx> {
-        // FIXME(min_const_generic_exprs): We could process obligations here if `ct` is a var.
+        let ct = self.resolve_vars_with_obligations(ct);
 
         if self.next_trait_solver()
             && let ty::ConstKind::Unevaluated(..) = ct.kind()
@@ -1510,6 +1513,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    pub(crate) fn structurally_resolve_const(
+        &self,
+        sp: Span,
+        ct: ty::Const<'tcx>,
+    ) -> ty::Const<'tcx> {
+        let ct = self.try_structurally_resolve_const(sp, ct);
+
+        if !ct.is_ct_infer() {
+            ct
+        } else {
+            let e = self.tainted_by_errors().unwrap_or_else(|| {
+                self.err_ctxt()
+                    .emit_inference_failure_err(
+                        self.body_id,
+                        sp,
+                        ct.into(),
+                        TypeAnnotationNeeded::E0282,
+                        true,
+                    )
+                    .emit()
+            });
+            // FIXME: Infer `?ct = {const error}`?
+            ty::Const::new_error(self.tcx, e)
+        }
+    }
+
     pub(crate) fn with_breakable_ctxt<F: FnOnce() -> R, R>(
         &self,
         id: HirId,

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -115,6 +115,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    pub(in super::super) fn check_repeat_exprs(&self) {
+        let mut deferred_repeat_expr_checks = self.deferred_repeat_expr_checks.borrow_mut();
+        debug!("FnCtxt::check_repeat_exprs: {} deferred checks", deferred_repeat_expr_checks.len());
+        for (element, element_ty, count) in deferred_repeat_expr_checks.drain(..) {
+            // We want to emit an error if the const is not structurally resolveable as otherwise
+            // we can find up conservatively proving `Copy` which may infer the repeat expr count
+            // to something that never required `Copy` in the first place.
+            let count =
+                self.structurally_resolve_const(element.span, self.normalize(element.span, count));
+
+            // Avoid run on "`NotCopy: Copy` is not implemented" errors when the repeat expr count
+            // is erroneous/unknown. The user might wind up specifying a repeat count of 0/1.
+            if count.references_error() {
+                continue;
+            }
+
+            // If the length is 0, we don't create any elements, so we don't copy any.
+            // If the length is 1, we don't copy that one element, we move it. Only check
+            // for `Copy` if the length is larger.
+            if count.try_to_target_usize(self.tcx).is_none_or(|x| x > 1) {
+                self.enforce_repeat_element_needs_copy_bound(element, element_ty);
+            }
+        }
+    }
+
     pub(in super::super) fn check_method_argument_types(
         &self,
         sp: Span,

compiler/rustc_hir_typeck/src/lib.rs

@@ -199,6 +199,15 @@ fn typeck_with_inspect<'tcx>(
         fcx.write_ty(id, expected_type);
     };
 
+    // Whether to check repeat exprs before/after inference fallback is somewhat arbitrary of a decision
+    // as neither option is strictly more permissive than the other. However, we opt to check repeat exprs
+    // first as errors from not having inferred array lengths yet seem less confusing than errors from inference
+    // fallback arbitrarily inferring something incompatible with `Copy` inference side effects.
+    //
+    // This should also be forwards compatible with moving repeat expr checks to a custom goal kind or using
+    // marker traits in the future.
+    fcx.check_repeat_exprs();
+
     fcx.type_inference_fallback();
 
     // Even though coercion casts provide type hints, we check casts after fallback for

compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs

@@ -62,6 +62,9 @@ pub(crate) struct TypeckRootCtxt<'tcx> {
 
     pub(super) deferred_coroutine_interiors: RefCell<Vec<(LocalDefId, hir::BodyId, Ty<'tcx>)>>,
 
+    pub(super) deferred_repeat_expr_checks:
+        RefCell<Vec<(&'tcx hir::Expr<'tcx>, Ty<'tcx>, ty::Const<'tcx>)>>,
+
     /// Whenever we introduce an adjustment from `!` into a type variable,
     /// we record that type variable here. This is later used to inform
     /// fallback. See the `fallback` module for details.
@@ -96,6 +99,7 @@ impl<'tcx> TypeckRootCtxt<'tcx> {
             deferred_transmute_checks: RefCell::new(Vec::new()),
             deferred_asm_checks: RefCell::new(Vec::new()),
             deferred_coroutine_interiors: RefCell::new(Vec::new()),
+            deferred_repeat_expr_checks: RefCell::new(Vec::new()),
             diverging_type_vars: RefCell::new(Default::default()),
             infer_var_info: RefCell::new(Default::default()),
         }

tests/ui/consts/const-fn-in-vec.rs

@@ -1,11 +1,16 @@
 static _MAYBE_STRINGS: [Option<String>; 5] = [None; 5];
 //~^ ERROR the trait bound `String: Copy` is not satisfied
 
-fn main() {
-    // should hint to create an inline `const` block
-    // or to create a new `const` item
+// should hint to create an inline `const` block
+// or to create a new `const` item
+fn foo() {
     let _strings: [String; 5] = [String::new(); 5];
     //~^ ERROR the trait bound `String: Copy` is not satisfied
+}
+
+fn bar() {
     let _maybe_strings: [Option<String>; 5] = [None; 5];
     //~^ ERROR the trait bound `String: Copy` is not satisfied
 }
+
+fn main() {}

tests/ui/consts/const-fn-in-vec.stderr

@@ -22,7 +22,7 @@ LL |     let _strings: [String; 5] = [String::new(); 5];
    = note: the `Copy` trait is required because this value will be copied for each element of the array
 
 error[E0277]: the trait bound `String: Copy` is not satisfied
-  --> $DIR/const-fn-in-vec.rs:9:48
+  --> $DIR/const-fn-in-vec.rs:12:48
    |
 LL |     let _maybe_strings: [Option<String>; 5] = [None; 5];
    |                                                ^^^^

tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs

@@ -0,0 +1,37 @@
+#![feature(generic_arg_infer)]
+
+// Test that would start passing if we defer repeat expr copy checks to end of
+// typechecking and they're checked after integer fallback occurs. We accomplish
+// this by contriving a situation where integer fallback allows progress to be
+// made on a trait goal that infers the length of a repeat expr.
+
+use std::marker::PhantomData;
+
+struct NotCopy;
+
+trait Trait<const N: usize> {}
+
+impl Trait<2> for u32 {}
+impl Trait<1> for i32 {}
+
+fn make_goal<T: Trait<N>, const N: usize>(_: &T, _: [NotCopy; N]) {}
+
+fn main() {
+    let a = 1;
+    let b = [NotCopy; _];
+    //~^ ERROR: type annotations needed
+
+    // a is of type `?y`
+    // b is of type `[NotCopy; ?x]`
+    // there is a goal ?y: Trait<?x>` with two candidates:
+    // - `i32: Trait<1>`, ?y=i32 ?x=1 which doesnt require `NotCopy: Copy`
+    // - `u32: Trait<2>` ?y=u32 ?x=2 which requires `NotCopy: Copy`
+    make_goal(&a, b);
+
+    // final repeat expr checks:
+    //
+    // `NotCopy; ?x`
+    // - succeeds if fallback happens before repeat exprs as `i32: Trait<?x>` infers `?x=1`
+    // - fails if repeat expr checks happen first as `?x` is unconstrained so cannot be
+    //    structurally resolved
+}

tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr

@@ -0,0 +1,14 @@
+error[E0282]: type annotations needed for `[NotCopy; _]`
+  --> $DIR/copy-check-deferred-after-fallback.rs:21:9
+   |
+LL |     let b = [NotCopy; _];
+   |         ^    ------- type must be known at this point
+   |
+help: consider giving `b` an explicit type, where the value of const parameter `N` is specified
+   |
+LL |     let b: [_; N] = [NotCopy; _];
+   |          ++++++++
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0282`.

tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs

@@ -0,0 +1,59 @@
+//@ check-pass
+
+#![feature(generic_arg_infer)]
+
+// Test that if we defer repeat expr copy checks to end of typechecking they're
+// checked before integer fallback occurs. We accomplish this by contriving a
+// situation where we have a goal that can be proven either via another repeat expr
+// check or by integer fallback. In the integer fallback case an array length would
+// be inferred to `2` requiring `NotCopy: Copy`, and in the repeat expr case it would
+// be inferred to `1`.
+
+use std::marker::PhantomData;
+
+struct NotCopy;
+
+struct Foo<T>(PhantomData<T>);
+
+impl Clone for Foo<u32> {
+    fn clone(&self) -> Self {
+        Foo(PhantomData)
+    }
+}
+
+impl Copy for Foo<u32> {}
+
+fn tie<T>(_: &T, _: [Foo<T>; 2]) {}
+
+trait Trait<const N: usize> {}
+
+impl Trait<2> for i32 {}
+impl Trait<1> for u32 {}
+
+fn make_goal<T: Trait<N>, const N: usize>(_: &T, _: [NotCopy; N]) {}
+
+fn main() {
+    let a = 1;
+    let b: [Foo<_>; 2] = [Foo(PhantomData); _];
+    tie(&a, b);
+    let c = [NotCopy; _];
+
+    // a is of type `?y`
+    // b is of type `[Foo<?y>; 2]`
+    // c is of type `[NotCopy; ?x]`
+    // there is a goal ?y: Trait<?x>` with two candidates:
+    // - `i32: Trait<2>`, ?y=i32 ?x=2 which requires `NotCopy: Copy` when expr checks happen
+    // - `u32: Trait<1>` ?y=u32 ?x=1 which doesnt require `NotCopy: Copy`
+    make_goal(&a, c);
+
+    // final repeat expr checks:
+    //
+    // `Foo<?y>; 2`
+    // - Foo<?y>: Copy
+    // - requires ?y=u32
+    //
+    // `NotCopy; ?x`
+    // - fails if fallback happens before repeat exprs as `i32: Trait<?x>` infers `?x=2`
+    // - succeeds if repeat expr checks happen first as `?y=u32` means `u32: Trait<?x>`
+    //    infers `?x=1`
+}

tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.gai.stderr

@@ -0,0 +1,17 @@
+error[E0282]: type annotations needed for `[Foo<_>; 2]`
+  --> $DIR/copy-inference-side-effects-are-lazy.rs:22:9
+   |
+LL |     let x = [Foo(PhantomData); 2];
+   |         ^
+LL |
+LL |     _ = extract(x).max(2);
+   |         ---------- type must be known at this point
+   |
+help: consider giving `x` an explicit type, where the type for type parameter `T` is specified
+   |
+LL |     let x: [Foo<T>; 2] = [Foo(PhantomData); 2];
+   |          +++++++++++++
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0282`.

tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs

@@ -0,0 +1,25 @@
+//@revisions: current gai
+//@[current] check-pass
+
+#![cfg_attr(gai, feature(generic_arg_infer))]
+
+use std::marker::PhantomData;
+
+struct Foo<T>(PhantomData<T>);
+
+impl Clone for Foo<u8> {
+    fn clone(&self) -> Self {
+        Foo(PhantomData)
+    }
+}
+impl Copy for Foo<u8> {}
+
+fn extract<T, const N: usize>(_: [Foo<T>; N]) -> T {
+    loop {}
+}
+
+fn main() {
+    let x = [Foo(PhantomData); 2];
+    //[gai]~^ ERROR: type annotations needed
+    _ = extract(x).max(2);
+}

tests/ui/repeat-expr/infer.rs renamed to tests/ui/repeat-expr/copy-inference-side-effects.rs

@@ -0,0 +1,6 @@
+//@ check-pass
+#![feature(generic_arg_infer)]
+
+fn main() {
+    let a: [_; 1] = [String::new(); _];
+}

tests/ui/repeat-expr/dont-require-copy-on-infer.rs

@@ -0,0 +1,20 @@
+#![feature(generic_arg_infer)]
+
+struct Foo<const N: usize>;
+
+impl Clone for Foo<1> {
+    fn clone(&self) -> Self {
+        Foo
+    }
+}
+impl Copy for Foo<1> {}
+
+fn unify<const N: usize>(_: &[Foo<N>; N]) {
+    loop {}
+}
+
+fn main() {
+    let x = &[Foo::<_>; _];
+    //~^ ERROR: type annotations needed for `&[Foo<_>; _]`
+    _ = unify(x);
+}