Skip to content

Commit cc10370

Browse files
committed
Reviews
1 parent 4316259 commit cc10370

File tree

4 files changed

+32
-25
lines changed

4 files changed

+32
-25
lines changed

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
108108
let deferred_repeat_expr_checks = deferred_repeat_expr_checks
109109
.drain(..)
110110
.flat_map(|(element, element_ty, count)| {
111-
// Actual constants as the repeat element get inserted repeatedly instead of getting copied via Copy
112-
// so we don't need to attempt to structurally resolve the repeat count which may unnecessarily error.
111+
// Actual constants as the repeat element are inserted repeatedly instead
112+
// of being copied via `Copy`, so we don't need to attempt to structurally
113+
// resolve the repeat count which may unnecessarily error.
113114
match &element.kind {
114115
hir::ExprKind::ConstBlock(..) => return None,
115116
hir::ExprKind::Path(qpath) => {
@@ -121,23 +122,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
121122
_ => {}
122123
}
123124

124-
// We want to emit an error if the const is not structurally resolveable as otherwise
125-
// we can find up conservatively proving `Copy` which may infer the repeat expr count
126-
// to something that never required `Copy` in the first place.
125+
// We want to emit an error if the const is not structurally resolveable
126+
// as otherwise we can wind up conservatively proving `Copy` which may
127+
// infer the repeat expr count to something that never required `Copy` in
128+
// the first place.
127129
let count = self
128130
.structurally_resolve_const(element.span, self.normalize(element.span, count));
129131

130-
// Avoid run on "`NotCopy: Copy` is not implemented" errors when the repeat expr count
131-
// is erroneous/unknown. The user might wind up specifying a repeat count of 0/1.
132+
// Avoid run on "`NotCopy: Copy` is not implemented" errors when the
133+
// repeat expr count is erroneous/unknown. The user might wind up
134+
// specifying a repeat count of 0/1.
132135
if count.references_error() {
133136
return None;
134137
}
135138

136139
Some((element, element_ty, count))
137140
})
138-
// We collect to force the side effects of structurally resolving the repeat count to happen in one
139-
// go, to avoid side effects from proving `Copy` affecting whether repeat counts are known or not.
140-
// If we did not do this we would get results that depend on the order that we evaluate each repeat
141+
// We collect to force the side effects of structurally resolving the repeat
142+
// count to happen in one go, to avoid side effects from proving `Copy`
143+
// affecting whether repeat counts are known or not. If we did not do this we
144+
// would get results that depend on the order that we evaluate each repeat
141145
// expr's `Copy` check.
142146
.collect::<Vec<_>>();
143147

@@ -171,14 +175,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
171175

172176
for (element, element_ty, count) in deferred_repeat_expr_checks {
173177
match count.kind() {
174-
ty::ConstKind::Value(val)
175-
if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) =>
176-
{
177-
enforce_copy_bound(element, element_ty)
178+
ty::ConstKind::Value(val) => {
179+
if val.try_to_target_usize(self.tcx).is_none_or(|count| count > 1) {
180+
enforce_copy_bound(element, element_ty)
181+
} else {
182+
// If the length is 0 or 1 we don't actually copy the element, we either don't create it
183+
// or we just use the one value.
184+
}
178185
}
179-
// If the length is 0 or 1 we don't actually copy the element, we either don't create it
180-
// or we just use the one value.
181-
ty::ConstKind::Value(_) => (),
182186

183187
// If the length is a generic parameter or some rigid alias then conservatively
184188
// require `element_ty: Copy` as it may wind up being `>1` after monomorphization.

compiler/rustc_hir_typeck/src/lib.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,13 +195,16 @@ fn typeck_with_inspect<'tcx>(
195195
fcx.write_ty(id, expected_type);
196196
};
197197

198-
// Whether to check repeat exprs before/after inference fallback is somewhat arbitrary of a decision
199-
// as neither option is strictly more permissive than the other. However, we opt to check repeat exprs
200-
// first as errors from not having inferred array lengths yet seem less confusing than errors from inference
201-
// fallback arbitrarily inferring something incompatible with `Copy` inference side effects.
198+
// Whether to check repeat exprs before/after inference fallback is somewhat
199+
// arbitrary of a decision as neither option is strictly more permissive than
200+
// the other. However, we opt to check repeat exprs first as errors from not
201+
// having inferred array lengths yet seem less confusing than errors from inference
202+
// fallback arbitrarily inferring something incompatible with `Copy` inference
203+
// side effects.
202204
//
203-
// This should also be forwards compatible with moving repeat expr checks to a custom goal kind or using
204-
// marker traits in the future.
205+
// FIXME(#140855): This should also be forwards compatible with moving
206+
// repeat expr checks to a custom goal kind or using marker traits in
207+
// the future.
205208
fcx.check_repeat_exprs();
206209

207210
fcx.type_inference_fallback();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// checked before integer fallback occurs. We accomplish this by having the repeat
66
// expr check allow inference progress on an ambiguous goal, where the ambiguous goal
77
// would fail if the inference variable was fallen back to `i32`. This test will
8-
// pass if wecheck repeat exprs before integer fallback.
8+
// pass if we check repeat exprs before integer fallback.
99

1010
use std::marker::PhantomData;
1111
struct Foo<T>(PhantomData<T>);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ impl Copy for Foo<1> {}
1212
fn unify<const N: usize>(_: &[Foo<N>; 2], _: &[String; N]) {}
1313

1414
fn works_if_inference_side_effects() {
15-
// This will only pass if inference side effectrs from proving `Foo<?x>: Copy` are
15+
// This will only pass if inference side effects from proving `Foo<?x>: Copy` are
1616
// able to be relied upon by other repeat expressions.
1717
let a /* : [Foo<?x>; 2] */ = [Foo::<_>; 2];
1818
//~^ ERROR: type annotations needed for `[Foo<_>; 2]`

0 commit comments

Comments
 (0)