Skip to content

Commit b84044a

Browse files
jackh726jackh726
committed
Review comments and more tests
1 parent f9e14af commit b84044a

File tree

3 files changed

+174
-11
lines changed

3 files changed

+174
-11
lines changed

‎compiler/rustc_typeck/src/check/wfcheck.rs

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,16 @@ pub fn check_trait_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
266266
/// Require that the user writes as where clauses on GATs the implicit
267267
/// outlives bounds involving trait parameters in trait functions and
268268
/// lifetimes passed as GAT substs. See `self-outlives-lint` test.
269+
///
270+
/// This trait will be our running example. We are currently WF checking the `Item` item...
271+
///
272+
/// ```rust
273+
/// trait LendingIterator {
274+
/// type Item<'me>; // <-- WF checking this trait item
275+
///
276+
/// fn next<'a>(&'a mut self) -> Option<Self::Item<'a>>;
277+
/// }
278+
/// ```
269279
fn check_gat_where_clauses(
270280
tcx: TyCtxt<'_>,
271281
trait_item: &hir::TraitItem<'_>,
@@ -282,28 +292,56 @@ fn check_gat_where_clauses(
282292
return;
283293
}
284294
let associated_items: &ty::AssocItems<'_> = tcx.associated_items(encl_trait_def_id);
285-
let mut clauses = FxHashSet::default();
295+
let mut clauses: Option<FxHashSet<ty::Predicate<'_>>> = None;
286296
// For every function in this trait...
297+
// In our example, this would be the `next` method
287298
for item in
288299
associated_items.in_definition_order().filter(|item| matches!(item.kind, ty::AssocKind::Fn))
289300
{
301+
// The clauses we that we would require from this function
302+
let mut function_clauses = FxHashSet::default();
303+
290304
let id = hir::HirId::make_owner(item.def_id.expect_local());
291305
let param_env = tcx.param_env(item.def_id.expect_local());
292306

293307
let sig = tcx.fn_sig(item.def_id);
308+
// Get the signature using placeholders. In our example, this would
309+
// convert the late-bound 'a into a free region.
294310
let sig = tcx.liberate_late_bound_regions(item.def_id, sig);
311+
// Collect the arguments that are given to this GAT in the return type
312+
// of the function signature. In our example, the GAT in the return
313+
// type is `<Self as LendingIterator>::Item<'a>`, so 'a and Self are arguments.
295314
let mut visitor = GATSubstCollector {
296315
tcx,
297316
gat: trait_item.def_id.to_def_id(),
298317
regions: FxHashSet::default(),
299318
types: FxHashSet::default(),
300319
};
301320
sig.output().visit_with(&mut visitor);
321+
322+
// If both regions and types are empty, then this GAT isn't in the
323+
// return type, and we shouldn't try to do clause analysis
324+
// (particularly, doing so would end up with an empty set of clauses,
325+
// since the current method would require none, and we take the
326+
// intersection of requirements of all methods)
327+
if visitor.types.is_empty() && visitor.regions.is_empty() {
328+
continue;
329+
}
330+
331+
// The types we can assume to be well-formed. In our example, this
332+
// would be &'a mut Self, from the first argument.
302333
let mut wf_tys = FxHashSet::default();
303334
wf_tys.extend(sig.inputs());
304335

336+
// For each region argument (e.g., 'a in our example), check for a
337+
// relationship to the type arguments (e.g., Self). If there is an
338+
// outlives relationship (`Self: 'a`), then we want to ensure that is
339+
// reflected in a where clause on the GAT itself.
305340
for (region, region_idx) in &visitor.regions {
306341
for (ty, ty_idx) in &visitor.types {
342+
// Unfortunately, we have to use a new `InferCtxt` for each
343+
// pair, because region constraints get added and solved there,
344+
// and we need to test each pair individually.
307345
tcx.infer_ctxt().enter(|infcx| {
308346
let mut outlives_environment = OutlivesEnvironment::new(param_env);
309347
outlives_environment.add_implied_bounds(&infcx, wf_tys.clone(), id, DUMMY_SP);
@@ -328,6 +366,7 @@ fn check_gat_where_clauses(
328366
Some(tcx.lifetimes.re_root_empty),
329367
param_env,
330368
);
369+
// In our example, requires that Self: 'a
331370
outlives.type_must_outlive(origin, sup_type, sub_region);
332371

333372
let errors = infcx.resolve_regions(
@@ -338,37 +377,57 @@ fn check_gat_where_clauses(
338377

339378
debug!(?errors, "errors");
340379

380+
// If we were able to prove that Self: 'a without an error,
381+
// it must be because of the implied or explicit bounds...
341382
if errors.is_empty() {
342383
debug!(?ty_idx, ?region_idx);
343384
debug!("required clause: {} must outlive {}", ty, region);
385+
// Translate into the generic parameters of the GAT. In
386+
// our example, the type was Self, which will also be
387+
// Self in the GAT.
344388
let ty_param = generics.param_at(*ty_idx, tcx);
345389
let ty_param = tcx.mk_ty(ty::Param(ty::ParamTy {
346390
index: ty_param.index,
347391
name: ty_param.name,
348392
}));
393+
// Same for the region. In our example, 'a corresponds
394+
// to the 'me parameter.
349395
let region_param = generics.param_at(*region_idx, tcx);
350396
let region_param =
351397
tcx.mk_region(ty::RegionKind::ReEarlyBound(ty::EarlyBoundRegion {
352398
def_id: region_param.def_id,
353399
index: region_param.index,
354400
name: region_param.name,
355401
}));
402+
// The predicate we expect to see. (In our example,
403+
// `Self: 'me`.)
356404
let clause = ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(
357405
ty_param,
358406
region_param,
359407
));
360408
let clause = tcx.mk_predicate(ty::Binder::dummy(clause));
361-
clauses.insert(clause);
409+
function_clauses.insert(clause);
362410
}
363411
});
364412
}
365413
}
414+
415+
match clauses.as_mut() {
416+
Some(clauses) => {
417+
clauses.drain_filter(|p| !function_clauses.contains(p));
418+
}
419+
None => {
420+
clauses = Some(function_clauses);
421+
}
422+
}
366423
}
367424

368425
// If there are any missing clauses, emit an error
426+
let mut clauses = clauses.unwrap_or_default();
369427
debug!(?clauses);
370428
if !clauses.is_empty() {
371-
let written_predicates: ty::GenericPredicates<'_> = tcx.predicates_of(trait_item.def_id);
429+
let written_predicates: ty::GenericPredicates<'_> =
430+
tcx.explicit_predicates_of(trait_item.def_id);
372431
let clauses: Vec<_> = clauses
373432
.drain_filter(|clause| {
374433
written_predicates.predicates.iter().find(|p| &p.0 == clause).is_none()
@@ -402,6 +461,10 @@ fn check_gat_where_clauses(
402461
}
403462
}
404463

464+
/// TypeVisitor that looks for uses of GATs like
465+
/// `<P0 as Trait<P1..Pn>>::GAT<Pn..Pm>` and adds the arguments `P0..Pm` into
466+
/// the two vectors, `regions` and `types` (depending on their kind). For each
467+
/// parameter `Pi` also track the index `i`.
405468
struct GATSubstCollector<'tcx> {
406469
tcx: TyCtxt<'tcx>,
407470
gat: DefId,

‎src/test/ui/generic-associated-types/self-outlives-lint.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
// check-fail
44

5+
use std::fmt::Debug;
6+
57
// We have a `&'a self`, so we need a `Self: 'a`
68
trait Iterable {
79
type Item<'x>;
@@ -100,4 +102,70 @@ impl Des3 for () {
100102
}
101103
*/
102104

105+
// Similar case to before, except with GAT.
106+
trait NoGat<'a> {
107+
type Bar;
108+
fn method(&'a self) -> Self::Bar;
109+
}
110+
111+
// Lifetime is not on function; except `Self: 'a`
112+
trait TraitLifetime<'a> {
113+
type Bar<'b>;
114+
//~^ Missing required bounds
115+
fn method(&'a self) -> Self::Bar<'a>;
116+
}
117+
118+
// Like above, but we have a where clause that can prove what we want
119+
trait TraitLifetimeWhere<'a> where Self: 'a {
120+
type Bar<'b>;
121+
//~^ Missing required bounds
122+
fn method(&'a self) -> Self::Bar<'a>;
123+
}
124+
125+
// Explicit bound instead of implicit; we want to still error
126+
trait ExplicitBound {
127+
type Bar<'b>;
128+
//~^ Missing required bounds
129+
fn method<'b>(&self, token: &'b ()) -> Self::Bar<'b> where Self: 'b;
130+
}
131+
132+
// The use of the GAT here is not in the return, we don't want to error
133+
trait NotInReturn {
134+
type Bar<'b>;
135+
fn method<'b>(&'b self) where Self::Bar<'b>: Debug;
136+
}
137+
138+
// We obviously error for `Iterator`, but we should also error for `Item`
139+
trait IterableTwo {
140+
type Item<'a>;
141+
type Iterator<'a>: Iterator<Item = Self::Item<'a>>;
142+
//~^ Missing required bounds
143+
fn iter<'a>(&'a self) -> Self::Iterator<'a>;
144+
}
145+
146+
// We also should report region outlives clauses
147+
trait RegionOutlives {
148+
type Bar<'a, 'b>;
149+
//~^ Missing required bounds
150+
fn foo<'x, 'y>(&self, input: &'x &'y ()) -> Self::Bar<'x, 'y>;
151+
}
152+
153+
/*
154+
impl Foo for () {
155+
type Bar<'a, 'b> = &'a &'b ();
156+
fn foo<'x, 'y>(&self, input: &'x &'y ()) -> Self::Bar<'x, 'y> {
157+
input
158+
}
159+
}
160+
*/
161+
162+
// If there are multiple methods that return the GAT, require a set of clauses
163+
// that can be satisfied by *all* methods
164+
trait MultipleMethods {
165+
type Bar<'me>;
166+
167+
fn gimme<'a>(&'a self) -> Self::Bar<'a>;
168+
fn gimme_default(&self) -> Self::Bar<'static>;
169+
}
170+
103171
fn main() {}

‎src/test/ui/generic-associated-types/self-outlives-lint.stderr

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,90 @@
11
error: Missing required bounds on Item
2-
--> $DIR/self-outlives-lint.rs:7:5
2+
--> $DIR/self-outlives-lint.rs:9:5
33
|
44
LL | type Item<'x>;
55
| ^^^^^^^^^^^^^-
66
| |
77
| help: add the required where clauses: `where Self: 'x`
88

99
error: Missing required bounds on Out
10-
--> $DIR/self-outlives-lint.rs:23:5
10+
--> $DIR/self-outlives-lint.rs:25:5
1111
|
1212
LL | type Out<'x>;
1313
| ^^^^^^^^^^^^-
1414
| |
1515
| help: add the required where clauses: `where T: 'x`
1616

1717
error: Missing required bounds on Out
18-
--> $DIR/self-outlives-lint.rs:37:5
18+
--> $DIR/self-outlives-lint.rs:39:5
1919
|
2020
LL | type Out<'x>;
2121
| ^^^^^^^^^^^^-
2222
| |
2323
| help: add the required where clauses: `where T: 'x`
2424

2525
error: Missing required bounds on Out
26-
--> $DIR/self-outlives-lint.rs:44:5
26+
--> $DIR/self-outlives-lint.rs:46:5
2727
|
2828
LL | type Out<'x, 'y>;
2929
| ^^^^^^^^^^^^^^^^-
3030
| |
3131
| help: add the required where clauses: `where U: 'y, T: 'x`
3232

3333
error: Missing required bounds on Out
34-
--> $DIR/self-outlives-lint.rs:59:5
34+
--> $DIR/self-outlives-lint.rs:61:5
3535
|
3636
LL | type Out<'x, D>;
3737
| ^^^^^^^^^^^^^^^-
3838
| |
3939
| help: add the required where clauses: `where D: 'x`
4040

4141
error: Missing required bounds on Out
42-
--> $DIR/self-outlives-lint.rs:75:5
42+
--> $DIR/self-outlives-lint.rs:77:5
4343
|
4444
LL | type Out<'x, D>;
4545
| ^^^^^^^^^^^^^^^-
4646
| |
4747
| help: add the required where clauses: `where D: 'x`
4848

4949
error: Missing required bounds on Out
50-
--> $DIR/self-outlives-lint.rs:90:5
50+
--> $DIR/self-outlives-lint.rs:92:5
5151
|
5252
LL | type Out<'x, D>;
5353
| ^^^^^^^^^^^^^^^-
5454
| |
5555
| help: add the required where clauses: `where D: 'x`
5656

57-
error: aborting due to 7 previous errors
57+
error: Missing required bounds on Bar
58+
--> $DIR/self-outlives-lint.rs:113:5
59+
|
60+
LL | type Bar<'b>;
61+
| ^^^^^^^^^^^^-
62+
| |
63+
| help: add the required where clauses: `where Self: 'b, Self: 'a`
64+
65+
error: Missing required bounds on Bar
66+
--> $DIR/self-outlives-lint.rs:120:5
67+
|
68+
LL | type Bar<'b>;
69+
| ^^^^^^^^^^^^-
70+
| |
71+
| help: add the required where clauses: `where Self: 'b, Self: 'a`
72+
73+
error: Missing required bounds on Bar
74+
--> $DIR/self-outlives-lint.rs:127:5
75+
|
76+
LL | type Bar<'b>;
77+
| ^^^^^^^^^^^^-
78+
| |
79+
| help: add the required where clauses: `where Self: 'b`
80+
81+
error: Missing required bounds on Iterator
82+
--> $DIR/self-outlives-lint.rs:141:5
83+
|
84+
LL | type Iterator<'a>: Iterator<Item = Self::Item<'a>>;
85+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
86+
| |
87+
| help: add the required where clauses: `where Self: 'a`
88+
89+
error: aborting due to 11 previous errors
5890

0 commit comments

Comments
 (0)