Skip to content
51 changes: 1 addition & 50 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1900,62 +1900,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>],
Expand Down
110 changes: 91 additions & 19 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::potentially_plural_count;
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_index::IndexVec;
Expand Down Expand Up @@ -104,24 +104,96 @@ 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 are inserted repeatedly instead
// of being 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() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to an explicit match instead of the to_target_usize().is_none_or() as the is_none_or felt like somewhat subtle logic given the soundness sensitive nature of how we should be handling each variant 🤔

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!()
}
}
}
}
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,16 @@ 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.
// 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.
// FIXME(#140855): 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();
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/lang-items/lang-item-generic-requirements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
20 changes: 17 additions & 3 deletions tests/ui/lang-items/lang-item-generic-requirements.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
72 changes: 72 additions & 0 deletions tests/ui/repeat-expr/copy-check-const-element-uninferred-count.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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`.
Loading
Loading