Skip to content

Commit a7d6408

Browse files
borsbors
committed
Auto merge of #96899 - oli-obk:closure_wf_check_bounds, r=nikomatsakis
Check that closures satisfy their where bounds fixes #53092 fixes #90409 based on #96736
2 parents 1c80ac0 + 7a4ac84 commit a7d6408

24 files changed

+325
-186
lines changed

Diff for: ‎compiler/rustc_borrowck/src/region_infer/opaque_types.rs

+1-106
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
use rustc_data_structures::fx::FxHashMap;
21
use rustc_data_structures::vec_map::VecMap;
32
use rustc_hir::def_id::DefId;
43
use rustc_hir::OpaqueTyOrigin;
54
use rustc_infer::infer::InferCtxt;
6-
use rustc_middle::ty::subst::GenericArgKind;
75
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, TyCtxt, TypeFoldable};
8-
use rustc_span::Span;
96
use rustc_trait_selection::opaque_types::InferCtxtExt;
107

118
use super::RegionInferenceContext;
@@ -107,21 +104,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
107104

108105
let opaque_type_key =
109106
OpaqueTypeKey { def_id: opaque_type_key.def_id, substs: universal_substs };
110-
let remapped_type = infcx.infer_opaque_definition_from_instantiation(
107+
let ty = infcx.infer_opaque_definition_from_instantiation(
111108
opaque_type_key,
112109
universal_concrete_type,
113110
origin,
114111
);
115-
let ty = if check_opaque_type_parameter_valid(
116-
infcx.tcx,
117-
opaque_type_key,
118-
origin,
119-
concrete_type.span,
120-
) {
121-
remapped_type
122-
} else {
123-
infcx.tcx.ty_error()
124-
};
125112
// Sometimes two opaque types are the same only after we remap the generic parameters
126113
// back to the opaque type definition. E.g. we may have `OpaqueType<X, Y>` mapped to `(X, Y)`
127114
// and `OpaqueType<Y, X>` mapped to `(Y, X)`, and those are the same, but we only know that
@@ -184,95 +171,3 @@ impl<'tcx> RegionInferenceContext<'tcx> {
184171
})
185172
}
186173
}
187-
188-
fn check_opaque_type_parameter_valid(
189-
tcx: TyCtxt<'_>,
190-
opaque_type_key: OpaqueTypeKey<'_>,
191-
origin: OpaqueTyOrigin,
192-
span: Span,
193-
) -> bool {
194-
match origin {
195-
// No need to check return position impl trait (RPIT)
196-
// because for type and const parameters they are correct
197-
// by construction: we convert
198-
//
199-
// fn foo<P0..Pn>() -> impl Trait
200-
//
201-
// into
202-
//
203-
// type Foo<P0...Pn>
204-
// fn foo<P0..Pn>() -> Foo<P0...Pn>.
205-
//
206-
// For lifetime parameters we convert
207-
//
208-
// fn foo<'l0..'ln>() -> impl Trait<'l0..'lm>
209-
//
210-
// into
211-
//
212-
// type foo::<'p0..'pn>::Foo<'q0..'qm>
213-
// fn foo<l0..'ln>() -> foo::<'static..'static>::Foo<'l0..'lm>.
214-
//
215-
// which would error here on all of the `'static` args.
216-
OpaqueTyOrigin::FnReturn(..) | OpaqueTyOrigin::AsyncFn(..) => return true,
217-
// Check these
218-
OpaqueTyOrigin::TyAlias => {}
219-
}
220-
let opaque_generics = tcx.generics_of(opaque_type_key.def_id);
221-
let mut seen_params: FxHashMap<_, Vec<_>> = FxHashMap::default();
222-
for (i, arg) in opaque_type_key.substs.iter().enumerate() {
223-
let arg_is_param = match arg.unpack() {
224-
GenericArgKind::Type(ty) => matches!(ty.kind(), ty::Param(_)),
225-
GenericArgKind::Lifetime(lt) if lt.is_static() => {
226-
tcx.sess
227-
.struct_span_err(span, "non-defining opaque type use in defining scope")
228-
.span_label(
229-
tcx.def_span(opaque_generics.param_at(i, tcx).def_id),
230-
"cannot use static lifetime; use a bound lifetime \
231-
instead or remove the lifetime parameter from the \
232-
opaque type",
233-
)
234-
.emit();
235-
return false;
236-
}
237-
GenericArgKind::Lifetime(lt) => {
238-
matches!(*lt, ty::ReEarlyBound(_) | ty::ReFree(_))
239-
}
240-
GenericArgKind::Const(ct) => matches!(ct.val(), ty::ConstKind::Param(_)),
241-
};
242-
243-
if arg_is_param {
244-
seen_params.entry(arg).or_default().push(i);
245-
} else {
246-
// Prevent `fn foo() -> Foo<u32>` from being defining.
247-
let opaque_param = opaque_generics.param_at(i, tcx);
248-
tcx.sess
249-
.struct_span_err(span, "non-defining opaque type use in defining scope")
250-
.span_note(
251-
tcx.def_span(opaque_param.def_id),
252-
&format!(
253-
"used non-generic {} `{}` for generic parameter",
254-
opaque_param.kind.descr(),
255-
arg,
256-
),
257-
)
258-
.emit();
259-
return false;
260-
}
261-
}
262-
263-
for (_, indices) in seen_params {
264-
if indices.len() > 1 {
265-
let descr = opaque_generics.param_at(indices[0], tcx).kind.descr();
266-
let spans: Vec<_> = indices
267-
.into_iter()
268-
.map(|i| tcx.def_span(opaque_generics.param_at(i, tcx).def_id))
269-
.collect();
270-
tcx.sess
271-
.struct_span_err(span, "non-defining opaque type use in defining scope")
272-
.span_note(spans, &format!("{} used multiple times", descr))
273-
.emit();
274-
return false;
275-
}
276-
}
277-
true
278-
}

Diff for: ‎compiler/rustc_trait_selection/src/opaque_types.rs

+101
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
8282
));
8383
debug!(?definition_ty);
8484

85+
if !check_opaque_type_parameter_valid(
86+
self.tcx,
87+
opaque_type_key,
88+
origin,
89+
instantiated_ty.span,
90+
) {
91+
return self.tcx.ty_error();
92+
}
93+
8594
// Only check this for TAIT. RPIT already supports `src/test/ui/impl-trait/nested-return-type2.rs`
8695
// on stable and we'd break that.
8796
if let OpaqueTyOrigin::TyAlias = origin {
@@ -148,6 +157,98 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
148157
}
149158
}
150159

160+
fn check_opaque_type_parameter_valid(
161+
tcx: TyCtxt<'_>,
162+
opaque_type_key: OpaqueTypeKey<'_>,
163+
origin: OpaqueTyOrigin,
164+
span: Span,
165+
) -> bool {
166+
match origin {
167+
// No need to check return position impl trait (RPIT)
168+
// because for type and const parameters they are correct
169+
// by construction: we convert
170+
//
171+
// fn foo<P0..Pn>() -> impl Trait
172+
//
173+
// into
174+
//
175+
// type Foo<P0...Pn>
176+
// fn foo<P0..Pn>() -> Foo<P0...Pn>.
177+
//
178+
// For lifetime parameters we convert
179+
//
180+
// fn foo<'l0..'ln>() -> impl Trait<'l0..'lm>
181+
//
182+
// into
183+
//
184+
// type foo::<'p0..'pn>::Foo<'q0..'qm>
185+
// fn foo<l0..'ln>() -> foo::<'static..'static>::Foo<'l0..'lm>.
186+
//
187+
// which would error here on all of the `'static` args.
188+
OpaqueTyOrigin::FnReturn(..) | OpaqueTyOrigin::AsyncFn(..) => return true,
189+
// Check these
190+
OpaqueTyOrigin::TyAlias => {}
191+
}
192+
let opaque_generics = tcx.generics_of(opaque_type_key.def_id);
193+
let mut seen_params: FxHashMap<_, Vec<_>> = FxHashMap::default();
194+
for (i, arg) in opaque_type_key.substs.iter().enumerate() {
195+
let arg_is_param = match arg.unpack() {
196+
GenericArgKind::Type(ty) => matches!(ty.kind(), ty::Param(_)),
197+
GenericArgKind::Lifetime(lt) if lt.is_static() => {
198+
tcx.sess
199+
.struct_span_err(span, "non-defining opaque type use in defining scope")
200+
.span_label(
201+
tcx.def_span(opaque_generics.param_at(i, tcx).def_id),
202+
"cannot use static lifetime; use a bound lifetime \
203+
instead or remove the lifetime parameter from the \
204+
opaque type",
205+
)
206+
.emit();
207+
return false;
208+
}
209+
GenericArgKind::Lifetime(lt) => {
210+
matches!(*lt, ty::ReEarlyBound(_) | ty::ReFree(_))
211+
}
212+
GenericArgKind::Const(ct) => matches!(ct.val(), ty::ConstKind::Param(_)),
213+
};
214+
215+
if arg_is_param {
216+
seen_params.entry(arg).or_default().push(i);
217+
} else {
218+
// Prevent `fn foo() -> Foo<u32>` from being defining.
219+
let opaque_param = opaque_generics.param_at(i, tcx);
220+
tcx.sess
221+
.struct_span_err(span, "non-defining opaque type use in defining scope")
222+
.span_note(
223+
tcx.def_span(opaque_param.def_id),
224+
&format!(
225+
"used non-generic {} `{}` for generic parameter",
226+
opaque_param.kind.descr(),
227+
arg,
228+
),
229+
)
230+
.emit();
231+
return false;
232+
}
233+
}
234+
235+
for (_, indices) in seen_params {
236+
if indices.len() > 1 {
237+
let descr = opaque_generics.param_at(indices[0], tcx).kind.descr();
238+
let spans: Vec<_> = indices
239+
.into_iter()
240+
.map(|i| tcx.def_span(opaque_generics.param_at(i, tcx).def_id))
241+
.collect();
242+
tcx.sess
243+
.struct_span_err(span, "non-defining opaque type use in defining scope")
244+
.span_note(spans, &format!("{} used multiple times", descr))
245+
.emit();
246+
return false;
247+
}
248+
}
249+
true
250+
}
251+
151252
struct ReverseMapper<'tcx> {
152253
tcx: TyCtxt<'tcx>,
153254

Diff for: ‎compiler/rustc_trait_selection/src/traits/wf.rs

+16-8
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
575575
// generators don't take arguments.
576576
}
577577

578-
ty::Closure(_, substs) => {
578+
ty::Closure(did, substs) => {
579579
// Only check the upvar types for WF, not the rest
580580
// of the types within. This is needed because we
581581
// capture the signature and it may not be WF
@@ -596,18 +596,26 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
596596
// probably always be WF, because it should be
597597
// shorthand for something like `where(T: 'a) {
598598
// fn(&'a T) }`, as discussed in #25860.
599-
//
600-
// Note that we are also skipping the generic
601-
// types. This is consistent with the `outlives`
602-
// code, but anyway doesn't matter: within the fn
599+
walker.skip_current_subtree(); // subtree handled below
600+
// FIXME(eddyb) add the type to `walker` instead of recursing.
601+
self.compute(substs.as_closure().tupled_upvars_ty().into());
602+
// Note that we cannot skip the generic types
603+
// types. Normally, within the fn
603604
// body where they are created, the generics will
604605
// always be WF, and outside of that fn body we
605606
// are not directly inspecting closure types
606607
// anyway, except via auto trait matching (which
607608
// only inspects the upvar types).
608-
walker.skip_current_subtree(); // subtree handled below
609-
// FIXME(eddyb) add the type to `walker` instead of recursing.
610-
self.compute(substs.as_closure().tupled_upvars_ty().into());
609+
// But when a closure is part of a type-alias-impl-trait
610+
// then the function that created the defining site may
611+
// have had more bounds available than the type alias
612+
// specifies. This may cause us to have a closure in the
613+
// hidden type that is not actually well formed and
614+
// can cause compiler crashes when the user abuses unsafe
615+
// code to procure such a closure.
616+
// See src/test/ui/type-alias-impl-trait/wf_check_closures.rs
617+
let obligations = self.nominal_obligations(did, substs);
618+
self.out.extend(obligations);
611619
}
612620

613621
ty::FnPtr(_) => {

Diff for: ‎src/test/ui/const-generics/generic_const_exprs/closures.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#![feature(generic_const_exprs)]
22
#![allow(incomplete_features)]
33
fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
4-
//~^ ERROR overly complex generic constant
4+
//~^ ERROR cycle detected when building an abstract representation
55

66
fn main() {}

Diff for: ‎src/test/ui/const-generics/generic_const_exprs/closures.stderr

+19-6
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
1-
error: overly complex generic constant
1+
error[E0391]: cycle detected when building an abstract representation for test::{constant#0}
22
--> $DIR/closures.rs:3:35
33
|
44
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
5-
| ^^^^-------^^
6-
| |
7-
| borrowing is not supported in generic constants
5+
| ^^^^^^^^^^^^^
86
|
9-
= help: consider moving this anonymous constant into a `const` function
10-
= note: this operation may be supported in the future
7+
note: ...which requires building THIR for `test::{constant#0}`...
8+
--> $DIR/closures.rs:3:35
9+
|
10+
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
11+
| ^^^^^^^^^^^^^
12+
note: ...which requires type-checking `test::{constant#0}`...
13+
--> $DIR/closures.rs:3:35
14+
|
15+
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
16+
| ^^^^^^^^^^^^^
17+
= note: ...which again requires building an abstract representation for test::{constant#0}, completing the cycle
18+
note: cycle used when checking that `test` is well-formed
19+
--> $DIR/closures.rs:3:1
20+
|
21+
LL | fn test<const N: usize>() -> [u8; N + (|| 42)()] {}
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1123

1224
error: aborting due to previous error
1325

26+
For more information about this error, try `rustc --explain E0391`.

Diff for: ‎src/test/ui/higher-rank-trait-bounds/issue-59311.nll.stderr

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: higher-ranked lifetime error
2+
--> $DIR/issue-59311.rs:17:5
3+
|
4+
LL | v.t(|| {});
5+
| ^^^^^^^^^^
6+
|
7+
= note: could not prove [closure@$DIR/issue-59311.rs:17:9: 17:14] well-formed
8+
9+
error: higher-ranked lifetime error
10+
--> $DIR/issue-59311.rs:17:9
11+
|
12+
LL | v.t(|| {});
13+
| ^^^^^
14+
|
15+
= note: could not prove for<'a> &'a V: 'static
16+
17+
error: aborting due to 2 previous errors
18+

Diff for: ‎src/test/ui/higher-rank-trait-bounds/issue-59311.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub fn crash<V>(v: &V)
1414
where
1515
for<'a> &'a V: T + 'static,
1616
{
17-
v.t(|| {}); //~ ERROR: higher-ranked lifetime error
17+
v.t(|| {}); //~ ERROR: `&'a V` does not fulfill the required lifetime
1818
}
1919

2020
fn main() {}

Diff for: ‎src/test/ui/higher-rank-trait-bounds/issue-59311.stderr

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
1-
error: higher-ranked lifetime error
2-
--> $DIR/issue-59311.rs:17:9
1+
error[E0477]: the type `&'a V` does not fulfill the required lifetime
2+
--> $DIR/issue-59311.rs:17:5
33
|
44
LL | v.t(|| {});
5-
| ^^^^^
5+
| ^^^^^^^^^^
66
|
7-
= note: could not prove for<'a> &'a V: 'static
7+
note: type must satisfy the static lifetime as required by this binding
8+
--> $DIR/issue-59311.rs:15:24
9+
|
10+
LL | for<'a> &'a V: T + 'static,
11+
| ^^^^^^^
812

913
error: aborting due to previous error
1014

15+
For more information about this error, try `rustc --explain E0477`.

Diff for: ‎src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.rs

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ type TwoConsts<const X: usize, const Y: usize> = impl Debug;
1515
fn one_ty<T: Debug>(t: T) -> TwoTys<T, T> {
1616
t
1717
//~^ ERROR non-defining opaque type use in defining scope
18-
//~| ERROR `U` doesn't implement `Debug`
1918
}
2019

2120
fn one_lifetime<'a>(t: &'a u32) -> TwoLifetimes<'a, 'a> {

0 commit comments

Comments
 (0)