Finalize repeat expr inference behaviour with inferred repeat counts

compiler/rustc_hir_typeck/src/expr.rs

@@ -1868,62 +1868,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // 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.
-        } 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);
-        }
+        self.deferred_repeat_expr_checks.borrow_mut().push((element, element_ty, count));
 
         let ty = Ty::new_array_with_const_len(tcx, t, count);
         self.register_wf_obligation(ty.into(), expr.span, ObligationCauseCode::WellFormed(None));
         ty
     }
 
-    /// Requires that `element_ty` is `Copy` (unless it's a const expression itself).
-    pub(super) fn enforce_repeat_element_needs_copy_bound(
-        &self,
-        element: &hir::Expr<'_>,
-        element_ty: Ty<'tcx>,
-    ) {
-        let tcx = self.tcx;
-        // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy.
-        match &element.kind {
-            hir::ExprKind::ConstBlock(..) => return,
-            hir::ExprKind::Path(qpath) => {
-                let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id);
-                if let Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::AnonConst, _) = res
-                {
-                    return;
-                }
-            }
-            _ => {}
-        }
-        // If someone calls a const fn or constructs a const value, they can extract that
-        // out into a separate constant (or a const block in the future), so we check that
-        // to tell them that in the diagnostic. Does not affect typeck.
-        let is_constable = match element.kind {
-            hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() {
-                ty::FnDef(def_id, _) if tcx.is_stable_const_fn(def_id) => traits::IsConstable::Fn,
-                _ => traits::IsConstable::No,
-            },
-            hir::ExprKind::Path(qpath) => {
-                match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) {
-                    Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor,
-                    _ => traits::IsConstable::No,
-                }
-            }
-            _ => traits::IsConstable::No,
-        };
-
-        let lang_item = self.tcx.require_lang_item(LangItem::Copy, None);
-        let code =
-            traits::ObligationCauseCode::RepeatElementCopy { is_constable, elt_span: element.span };
-        self.require_type_meets(element_ty, element.span, code, lang_item);
-    }
-
     fn check_expr_tuple(
         &self,
         elts: &'tcx [hir::Expr<'tcx>],

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -4,10 +4,10 @@ use itertools::Itertools;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::codes::*;
 use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan, a_or_an, listify, pluralize};
-use rustc_hir::def::{CtorOf, DefKind, Res};
+use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
-use rustc_hir::{ExprKind, HirId, Node, QPath};
+use rustc_hir::{ExprKind, HirId, LangItem, Node, QPath};
 use rustc_hir_analysis::check::intrinsicck::InlineAsmCtxt;
 use rustc_hir_analysis::check::potentially_plural_count;
 use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
@@ -111,24 +111,92 @@ 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);
+        let deferred_repeat_expr_checks = deferred_repeat_expr_checks
+            .drain(..)
+            .flat_map(|(element, element_ty, count)| {
+                // Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy
+                // so we don't need to attempt to structurally resolve the repeat count which may unnecessarily error.
+                match &element.kind {
+                    hir::ExprKind::ConstBlock(..) => return None,
+                    hir::ExprKind::Path(qpath) => {
+                        let res = self.typeck_results.borrow().qpath_res(qpath, element.hir_id);
+                        if let Res::Def(DefKind::Const | DefKind::AssocConst, _) = res {
+                            return None;
+                        }
+                    }
+                    _ => {}
+                }
+
+                // We want to emit an error if the const is not structurally resolveable as otherwise
+                // we can wind 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() {
+                    return None;
+                }
+
+                Some((element, element_ty, count))
+            })
+            // We collect to force the side effects of structurally resolving the repeat count to happen in one
+            // go, to avoid side effects from proving `Copy` affecting whether repeat counts are known or not.
+            // If we did not do this we would get results that depend on the order that we evaluate each repeat
+            // expr's `Copy` check.
+            .collect::<Vec<_>>();
+
+        let enforce_copy_bound = |element: &hir::Expr<'_>, element_ty| {
+            // If someone calls a const fn or constructs a const value, they can extract that
+            // out into a separate constant (or a const block in the future), so we check that
+            // to tell them that in the diagnostic. Does not affect typeck.
+            let is_constable = match element.kind {
+                hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() {
+                    ty::FnDef(def_id, _) if self.tcx.is_stable_const_fn(def_id) => {
+                        traits::IsConstable::Fn
+                    }
+                    _ => traits::IsConstable::No,
+                },
+                hir::ExprKind::Path(qpath) => {
+                    match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) {
+                        Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor,
+                        _ => traits::IsConstable::No,
+                    }
+                }
+                _ => traits::IsConstable::No,
+            };
+
+            let lang_item = self.tcx.require_lang_item(LangItem::Copy, None);
+            let code = traits::ObligationCauseCode::RepeatElementCopy {
+                is_constable,
+                elt_span: element.span,
+            };
+            self.require_type_meets(element_ty, element.span, code, lang_item);
+        };
+
+        for (element, element_ty, count) in deferred_repeat_expr_checks {
+            match count.kind() {
+                ty::ConstKind::Value(val) => {
+                    if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) {
+                        enforce_copy_bound(element, element_ty)
+                    } else {
+                        // If the length is 0 or 1 we don't actually copy the element, we either don't create it
+                        // or we just use the one value.
+                    }
+                }
+
+                // If the length is a generic parameter or some rigid alias then conservatively
+                // require `element_ty: Copy` as it may wind up being `>1` after monomorphization.
+                ty::ConstKind::Param(_)
+                | ty::ConstKind::Expr(_)
+                | ty::ConstKind::Placeholder(_)
+                | ty::ConstKind::Unevaluated(_) => enforce_copy_bound(element, element_ty),
+
+                ty::ConstKind::Bound(_, _) | ty::ConstKind::Infer(_) | ty::ConstKind::Error(_) => {
+                    unreachable!()
+                }
             }
         }
     }

tests/ui/lang-items/lang-item-generic-requirements.rs

@@ -49,12 +49,14 @@ fn ice() {
     // Use index
     let arr = [0; 5];
     let _ = arr[2];
+    //~^ ERROR cannot index into a value of type `[{integer}; 5]`
 
     // Use phantomdata
     let _ = MyPhantomData::<(), i32>;
 
     // Use Foo
     let _: () = Foo;
+    //~^ ERROR mismatched types
 }
 
 // use `start`

tests/ui/lang-items/lang-item-generic-requirements.stderr

@@ -76,9 +76,23 @@ LL |     r + a;
    |     |
    |     {integer}
 
+error[E0608]: cannot index into a value of type `[{integer}; 5]`
+  --> $DIR/lang-item-generic-requirements.rs:51:16
+   |
+LL |     let _ = arr[2];
+   |                ^^^
+
+error[E0308]: mismatched types
+  --> $DIR/lang-item-generic-requirements.rs:58:17
+   |
+LL |     let _: () = Foo;
+   |            --   ^^^ expected `()`, found `Foo`
+   |            |
+   |            expected due to this
+
 error: requires `copy` lang_item
 
-error: aborting due to 10 previous errors
+error: aborting due to 12 previous errors
 
-Some errors have detailed explanations: E0369, E0392, E0718.
-For more information about an error, try `rustc --explain E0369`.
+Some errors have detailed explanations: E0308, E0369, E0392, E0608, E0718.
+For more information about an error, try `rustc --explain E0308`.

tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs

@@ -0,0 +1,72 @@
+#![feature(generic_arg_infer)]
+
+// Test when deferring repeat expr copy checks to end of typechecking whether elements
+// that are const items allow for repeat counts to go uninferred without an error being
+// emitted if they would later wind up inferred by integer fallback.
+//
+// This test should be updated if we wind up deferring repeat expr checks until *after*
+// integer fallback as the point of the test is not *specifically* about integer fallback
+// but rather about the behaviour of `const` element exprs.
+
+trait Trait<const N: usize> {}
+
+// We impl `Trait` for both `i32` and `u32` to avoid being able
+// to prove `?int: Trait<?n>` from there only being one impl.
+impl Trait<2> for i32 {}
+impl Trait<2> for u32 {}
+
+fn tie_and_make_goal<const N: usize, T: Trait<N>>(_: &T, _: &[String; N]) {}
+
+fn const_block() {
+    // Deferred repeat expr `String; ?n`
+    let a = [const { String::new() }; _];
+
+    // `?int: Trait<?n>` goal
+    tie_and_make_goal(&1, &a);
+
+    // If repeat expr checks structurally resolve the `?n`s before checking if the
+    // element is a `const` then we would error here. Otherwise we avoid doing so,
+    // integer fallback occurs, allowing `?int: Trait<?n>` goals to make progress,
+    // inferring the repeat counts (to `2` but that doesn't matter as the element is `const`).
+}
+
+fn const_item() {
+    const MY_CONST: String = String::new();
+
+    // Deferred repeat expr `String; ?n`
+    let a = [MY_CONST; _];
+
+    // `?int: Trait<?n>` goal
+    tie_and_make_goal(&1, &a);
+
+    // ... same as `const_block`
+}
+
+fn assoc_const() {
+    trait Dummy {
+        const ASSOC: String;
+    }
+    impl Dummy for () {
+        const ASSOC: String = String::new();
+    }
+
+    // Deferred repeat expr `String; ?n`
+    let a = [<() as Dummy>::ASSOC; _];
+
+    // `?int: Trait<?n>` goal
+    tie_and_make_goal(&1, &a);
+
+    // ... same as `const_block`
+}
+
+fn const_block_but_uninferred() {
+    // Deferred repeat expr `String; ?n`
+    let a = [const { String::new() }; _];
+    //~^ ERROR: type annotations needed for `[String; _]`
+
+    // Even if we don't structurally resolve the repeat count as part of repeat expr
+    // checks, we still error on the repeat count being uninferred as we require all
+    // types/consts to be inferred by the end of type checking.
+}
+
+fn main() {}

tests/ui/repeat-expr/copy-check-const-element-uninferred-count.stderr

@@ -0,0 +1,15 @@
+error[E0284]: type annotations needed for `[String; _]`
+  --> $DIR/copy-check-const-element-uninferred-count.rs:64:9
+   |
+LL |     let a = [const { String::new() }; _];
+   |         ^   ---------------------------- type must be known at this point
+   |
+   = note: the length of array `[String; _]` must be type `usize`
+help: consider giving `a` an explicit type, where the placeholders `_` are specified
+   |
+LL |     let a: [_; _] = [const { String::new() }; _];
+   |          ++++++++
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0284`.

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

@@ -1,37 +1,53 @@
 #![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.
+// Test when deferring repeat expr copy checks to end of typechecking whether they're
+// checked before integer fallback occurs or not. We accomplish this by having a repeat
+// count that can only be inferred after integer fallback has occured. This test will
+// pass if we were to check repeat exprs after integer fallback.
 
 use std::marker::PhantomData;
-
-struct NotCopy;
+struct Foo<T>(PhantomData<T>);
+
+// We impl Copy/Clone for multiple (but not all) substitutions
+// to ensure that `Foo<?int>: Copy` can't be proven on the basis
+// of there only being one applying impl.
+impl Clone for Foo<u32> {
+    fn clone(&self) -> Self {
+        Foo(PhantomData)
+    }
+}
+impl Clone for Foo<i32> {
+    fn clone(&self) -> Self {
+        Foo(PhantomData)
+    }
+}
+impl Copy for Foo<u32> {}
+impl Copy for Foo<i32> {}
 
 trait Trait<const N: usize> {}
 
-impl Trait<2> for u32 {}
+// We impl `Trait` for both `i32` and `u32` to avoid being able
+// to prove `?int: Trait<?n>` from there only being one impl.
 impl Trait<1> for i32 {}
+impl Trait<2> for u32 {}
 
-fn make_goal<T: Trait<N>, const N: usize>(_: &T, _: [NotCopy; N]) {}
+fn tie_and_make_goal<const N: usize, T: Trait<N>>(_: &T, _: &[Foo<T>; 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
+    // Deferred repeat expr `Foo<?int>; ?n`
+    let b = [Foo(PhantomData); _];
+    //~^ ERROR: type annotations needed for `[Foo<{integer}>; _]`
+
+    // Introduces a `?int: Trait<?n>` goal
+    tie_and_make_goal(&a, &b);
+
+    // If fallback doesn't occur:
+    // - `Foo<?int>; ?n`is ambig as repeat count is unknown -> error
+
+    // If fallback occurs:
+    // - `?int` inferred to `i32`
+    // - `?int: Trait<?n>` becomes `i32: Trait<?n>` wihhc infers `?n=1`
+    // - Repeat expr check `Foo<?int>; ?n` is now `Foo<i32>; 1`
+    // - `Foo<i32>; 1` doesn't require `Foo<i32>: Copy`
 }