Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer repeat expr Copy checks to end of type checking #137045

Merged
merged 3 commits into from
Mar 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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>,
Expand Down
51 changes: 40 additions & 11 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Comment on lines +1537 to +1538
Copy link
Member Author

Choose a reason for hiding this comment

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

There's no fcx.demand_subtype for consts yet.

Copy link
Member

Choose a reason for hiding this comment

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

You could just self.at(..).eq(ct, err)?

}
}

pub(crate) fn with_breakable_ctxt<F: FnOnce() -> R, R>(
&self,
id: HirId,
Expand Down
25 changes: 25 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 feel somewhat hesitant to add a custom goal kind for repeat expr checks but I do think it would be "optimal" behaviour-wise as the goals could stall on the repeat count infer var and be deferred ~as long as necessary while also not delaying inference constraints from them any later than is needed.

// 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
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()),
}
Expand Down
11 changes: 8 additions & 3 deletions tests/ui/consts/const-fn-in-vec.rs
Original file line number Diff line number Diff line change
@@ -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() {}
2 changes: 1 addition & 1 deletion tests/ui/consts/const-fn-in-vec.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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];
| ^^^^
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/repeat-expr/copy-check-deferred-after-fallback.rs
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 14 additions & 0 deletions tests/ui/repeat-expr/copy-check-deferred-after-fallback.stderr
Original file line number Diff line number Diff line change
@@ -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`.
59 changes: 59 additions & 0 deletions tests/ui/repeat-expr/copy-check-deferred-before-fallback.rs
Original file line number Diff line number Diff line change
@@ -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`
}
Original file line number Diff line number Diff line change
@@ -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`.
25 changes: 25 additions & 0 deletions tests/ui/repeat-expr/copy-inference-side-effects-are-lazy.rs
Original file line number Diff line number Diff line change
@@ -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);
}
6 changes: 6 additions & 0 deletions tests/ui/repeat-expr/dont-require-copy-on-infer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//@ check-pass
#![feature(generic_arg_infer)]

fn main() {
let a: [_; 1] = [String::new(); _];
}
20 changes: 20 additions & 0 deletions tests/ui/repeat-expr/no-conservative-copy-impl-requirement.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Loading
Loading