Skip to content

Commit 3eeb5a6

Browse files
borsbors
committed
Auto merge of #46785 - leodasvacas:type-check-defaults-at-declaration, r=nikomatsakis
[Underspecified semantics] Type check defaults at declaration. Fixes #46669. See the test for code that compiles on stable but will no longer compile. This falls under a "Underspecified language semantics" fix. **Needs crater**. On type and trait declarations, we currently allow anything that name checks as a type parameter default. That allows the user to write a default that can never be applied, or even a default that may conditionally be applied depending on the type of another parameter. Mostly this just defers the error to use sites, but also allows clever hacks such as `Foo<T, U = <T as Iterator>::Item>` where `U` will be able to apply it's default only when `T: Iterator`. Maybe that means this bug is a feature, but it's a fiddly behaviour that seems undesirable. This PR validates defaults at declaration sites by ensuring all predicates on the parameter are valid for the default. With the exception of `Self: Sized` which we don't want to check to allow things like `trait Add<RHS = Self>`.
2 parents a85417f + 3e84aed commit 3eeb5a6

File tree

5 files changed

+231
-23
lines changed

5 files changed

+231
-23
lines changed

‎src/librustc_typeck/check/wfcheck.rs

+88-23
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
185185
reject_shadowing_type_parameters(fcx.tcx, item.def_id);
186186
let sig = fcx.tcx.fn_sig(item.def_id);
187187
let sig = fcx.normalize_associated_types_in(span, &sig);
188-
let predicates = fcx.tcx.predicates_of(item.def_id)
189-
.instantiate_identity(fcx.tcx);
190-
let predicates = fcx.normalize_associated_types_in(span, &predicates);
191-
this.check_fn_or_method(fcx, span, sig, &predicates,
188+
this.check_fn_or_method(fcx, span, sig,
192189
item.def_id, &mut implied_bounds);
193190
let sig_if_method = sig_if_method.expect("bad signature for method");
194191
this.check_method_receiver(fcx, sig_if_method, &item, self_ty);
@@ -272,20 +269,16 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
272269
}
273270
}
274271

275-
let predicates = fcx.tcx.predicates_of(def_id).instantiate_identity(fcx.tcx);
276-
let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
277-
this.check_where_clauses(fcx, item.span, &predicates);
272+
self.check_where_clauses(fcx, item.span, def_id);
278273

279274
vec![] // no implied bounds in a struct def'n
280275
});
281276
}
282277

283278
fn check_trait(&mut self, item: &hir::Item) {
284279
let trait_def_id = self.tcx.hir.local_def_id(item.id);
285-
self.for_item(item).with_fcx(|fcx, this| {
286-
let predicates = fcx.tcx.predicates_of(trait_def_id).instantiate_identity(fcx.tcx);
287-
let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
288-
this.check_where_clauses(fcx, item.span, &predicates);
280+
self.for_item(item).with_fcx(|fcx, _| {
281+
self.check_where_clauses(fcx, item.span, trait_def_id);
289282
vec![]
290283
});
291284
}
@@ -295,12 +288,8 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
295288
let def_id = fcx.tcx.hir.local_def_id(item.id);
296289
let sig = fcx.tcx.fn_sig(def_id);
297290
let sig = fcx.normalize_associated_types_in(item.span, &sig);
298-
299-
let predicates = fcx.tcx.predicates_of(def_id).instantiate_identity(fcx.tcx);
300-
let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
301-
302291
let mut implied_bounds = vec![];
303-
this.check_fn_or_method(fcx, item.span, sig, &predicates,
292+
this.check_fn_or_method(fcx, item.span, sig,
304293
def_id, &mut implied_bounds);
305294
implied_bounds
306295
})
@@ -354,19 +343,96 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
354343
}
355344
}
356345

357-
let predicates = fcx.tcx.predicates_of(item_def_id).instantiate_identity(fcx.tcx);
358-
let predicates = fcx.normalize_associated_types_in(item.span, &predicates);
359-
this.check_where_clauses(fcx, item.span, &predicates);
346+
this.check_where_clauses(fcx, item.span, item_def_id);
360347

361348
fcx.impl_implied_bounds(item_def_id, item.span)
362349
});
363350
}
364351

352+
/// Checks where clauses and inline bounds that are declared on def_id.
365353
fn check_where_clauses<'fcx, 'tcx>(&mut self,
366354
fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
367355
span: Span,
368-
predicates: &ty::InstantiatedPredicates<'tcx>)
369-
{
356+
def_id: DefId) {
357+
use ty::subst::Subst;
358+
use rustc::ty::TypeFoldable;
359+
360+
let mut predicates = fcx.tcx.predicates_of(def_id);
361+
let mut substituted_predicates = Vec::new();
362+
363+
let generics = self.tcx.generics_of(def_id);
364+
let is_our_default = |def: &ty::TypeParameterDef|
365+
def.has_default && def.index >= generics.parent_count() as u32;
366+
367+
// Check that concrete defaults are well-formed. See test `type-check-defaults.rs`.
368+
// For example this forbids the declaration:
369+
// struct Foo<T = Vec<[u32]>> { .. }
370+
// Here the default `Vec<[u32]>` is not WF because `[u32]: Sized` does not hold.
371+
for d in generics.types.iter().cloned().filter(is_our_default).map(|p| p.def_id) {
372+
let ty = fcx.tcx.type_of(d);
373+
// ignore dependent defaults -- that is, where the default of one type
374+
// parameter includes another (e.g., <T, U = T>). In those cases, we can't
375+
// be sure if it will error or not as user might always specify the other.
376+
if !ty.needs_subst() {
377+
fcx.register_wf_obligation(ty, fcx.tcx.def_span(d), self.code.clone());
378+
}
379+
}
380+
381+
// Check that trait predicates are WF when params are substituted by their defaults.
382+
// We don't want to overly constrain the predicates that may be written but we want to
383+
// catch cases where a default my never be applied such as `struct Foo<T: Copy = String>`.
384+
// Therefore we check if a predicate which contains a single type param
385+
// with a concrete default is WF with that default substituted.
386+
// For more examples see tests `defaults-well-formedness.rs` and `type-check-defaults.rs`.
387+
//
388+
// First we build the defaulted substitution.
389+
let substs = ty::subst::Substs::for_item(fcx.tcx, def_id, |def, _| {
390+
// All regions are identity.
391+
fcx.tcx.mk_region(ty::ReEarlyBound(def.to_early_bound_region_data()))
392+
}, |def, _| {
393+
// If the param has a default,
394+
if is_our_default(def) {
395+
let default_ty = fcx.tcx.type_of(def.def_id);
396+
// and it's not a dependent default
397+
if !default_ty.needs_subst() {
398+
// then substitute with the default.
399+
return default_ty;
400+
}
401+
}
402+
// Mark unwanted params as err.
403+
fcx.tcx.types.err
404+
});
405+
// Now we build the substituted predicates.
406+
for &pred in predicates.predicates.iter() {
407+
struct CountParams { params: FxHashSet<u32> }
408+
impl<'tcx> ty::fold::TypeVisitor<'tcx> for CountParams {
409+
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
410+
match t.sty {
411+
ty::TyParam(p) => {
412+
self.params.insert(p.idx);
413+
t.super_visit_with(self)
414+
}
415+
_ => t.super_visit_with(self)
416+
}
417+
}
418+
}
419+
let mut param_count = CountParams { params: FxHashSet() };
420+
pred.visit_with(&mut param_count);
421+
let substituted_pred = pred.subst(fcx.tcx, substs);
422+
// Don't check non-defaulted params, dependent defaults or preds with multiple params.
423+
if substituted_pred.references_error() || param_count.params.len() > 1 {
424+
continue;
425+
}
426+
// Avoid duplication of predicates that contain no parameters, for example.
427+
if !predicates.predicates.contains(&substituted_pred) {
428+
substituted_predicates.push(substituted_pred);
429+
}
430+
}
431+
432+
predicates.predicates.extend(substituted_predicates);
433+
let predicates = predicates.instantiate_identity(fcx.tcx);
434+
let predicates = fcx.normalize_associated_types_in(span, &predicates);
435+
370436
let obligations =
371437
predicates.predicates
372438
.iter()
@@ -385,7 +451,6 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
385451
fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
386452
span: Span,
387453
sig: ty::PolyFnSig<'tcx>,
388-
predicates: &ty::InstantiatedPredicates<'tcx>,
389454
def_id: DefId,
390455
implied_bounds: &mut Vec<Ty<'tcx>>)
391456
{
@@ -402,7 +467,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
402467
// FIXME(#25759) return types should not be implied bounds
403468
implied_bounds.push(sig.output());
404469

405-
self.check_where_clauses(fcx, span, predicates);
470+
self.check_where_clauses(fcx, span, def_id);
406471
}
407472

408473
fn check_method_receiver<'fcx, 'tcx>(&mut self,

‎src/test/run-pass/defaults-well-formedness.rs

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
trait Trait<T> {}
12+
struct Foo<U, V=i32>(U, V) where U: Trait<V>;
13+
14+
trait Marker {}
15+
struct TwoParams<T, U>(T, U);
16+
impl Marker for TwoParams<i32, i32> {}
17+
18+
// Clauses with more than 1 param are not checked.
19+
struct IndividuallyBogus<T = i32, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
20+
struct BogusTogether<T = u32, U = i32>(T, U) where TwoParams<T, U>: Marker;
21+
// Clauses with non-defaulted params are not checked.
22+
struct NonDefaultedInClause<T, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
23+
struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
24+
// Dependent defaults are not checked.
25+
struct Dependent<T, U = T>(T, U) where U: Copy;
26+
trait SelfBound<T: Copy=Self> {}
27+
// Not even for well-formedness.
28+
struct WellFormedProjection<A, T=<A as Iterator>::Item>(A, T);
29+
30+
fn main() {}

‎src/test/run-pass/type-macros-simple.rs

+1
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,4 @@ fn issue_36540() {
3434
}
3535

3636
trait Trait<T> {}
37+
impl Trait<i32> for i32 {}

‎src/test/ui/type-check-defaults.rs

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::iter::FromIterator;
12+
use std::vec::IntoIter;
13+
use std::ops::Add;
14+
15+
struct Foo<T, U: FromIterator<T>>(T, U);
16+
struct WellFormed<Z = Foo<i32, i32>>(Z);
17+
//~^ error: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied [E0277]
18+
struct WellFormedNoBounds<Z:?Sized = Foo<i32, i32>>(Z);
19+
//~^ error: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied [E0277]
20+
21+
struct Bounds<T:Copy=String>(T);
22+
//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
23+
24+
struct WhereClause<T=String>(T) where T: Copy;
25+
//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
26+
27+
trait TraitBound<T:Copy=String> {}
28+
//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
29+
30+
trait Super<T: Copy> { }
31+
trait Base<T = String>: Super<T> { }
32+
//~^ error: the trait bound `T: std::marker::Copy` is not satisfied [E0277]
33+
34+
trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
35+
//~^ error: cannot add `u8` to `i32` [E0277]
36+
37+
fn main() { }

‎src/test/ui/type-check-defaults.stderr

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
error[E0277]: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied
2+
--> $DIR/type-check-defaults.rs:16:19
3+
|
4+
LL | struct WellFormed<Z = Foo<i32, i32>>(Z);
5+
| ^ a collection of type `i32` cannot be built from an iterator over elements of type `i32`
6+
|
7+
= help: the trait `std::iter::FromIterator<i32>` is not implemented for `i32`
8+
note: required by `Foo`
9+
--> $DIR/type-check-defaults.rs:15:1
10+
|
11+
LL | struct Foo<T, U: FromIterator<T>>(T, U);
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13+
14+
error[E0277]: the trait bound `i32: std::iter::FromIterator<i32>` is not satisfied
15+
--> $DIR/type-check-defaults.rs:18:27
16+
|
17+
LL | struct WellFormedNoBounds<Z:?Sized = Foo<i32, i32>>(Z);
18+
| ^ a collection of type `i32` cannot be built from an iterator over elements of type `i32`
19+
|
20+
= help: the trait `std::iter::FromIterator<i32>` is not implemented for `i32`
21+
note: required by `Foo`
22+
--> $DIR/type-check-defaults.rs:15:1
23+
|
24+
LL | struct Foo<T, U: FromIterator<T>>(T, U);
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26+
27+
error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
28+
--> $DIR/type-check-defaults.rs:21:1
29+
|
30+
LL | struct Bounds<T:Copy=String>(T);
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
32+
|
33+
= note: required by `std::marker::Copy`
34+
35+
error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
36+
--> $DIR/type-check-defaults.rs:24:1
37+
|
38+
LL | struct WhereClause<T=String>(T) where T: Copy;
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
40+
|
41+
= note: required by `std::marker::Copy`
42+
43+
error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
44+
--> $DIR/type-check-defaults.rs:27:1
45+
|
46+
LL | trait TraitBound<T:Copy=String> {}
47+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
48+
|
49+
= note: required by `std::marker::Copy`
50+
51+
error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied
52+
--> $DIR/type-check-defaults.rs:31:1
53+
|
54+
LL | trait Base<T = String>: Super<T> { }
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `T`
56+
|
57+
= help: consider adding a `where T: std::marker::Copy` bound
58+
note: required by `Super`
59+
--> $DIR/type-check-defaults.rs:30:1
60+
|
61+
LL | trait Super<T: Copy> { }
62+
| ^^^^^^^^^^^^^^^^^^^^
63+
64+
error[E0277]: cannot add `u8` to `i32`
65+
--> $DIR/type-check-defaults.rs:34:1
66+
|
67+
LL | trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `i32 + u8`
69+
|
70+
= help: the trait `std::ops::Add<u8>` is not implemented for `i32`
71+
= note: required by `std::ops::Add`
72+
73+
error: aborting due to 7 previous errors
74+
75+
If you want more information on this error, try using "rustc --explain E0277"

0 commit comments

Comments
 (0)