Skip to content

Commit c74f85f

Browse files
committed
Check only predicates with a single param with a concrete default.
This is the most conservative possible and should be always correct.
1 parent ac17948 commit c74f85f

File tree

4 files changed

+49
-101
lines changed

4 files changed

+49
-101
lines changed

src/librustc_typeck/check/wfcheck.rs

Lines changed: 31 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
278278
fn check_trait(&mut self, item: &hir::Item) {
279279
let trait_def_id = self.tcx.hir.local_def_id(item.id);
280280
self.for_item(item).with_fcx(|fcx, _| {
281-
self.check_trait_where_clauses(fcx, item.span, trait_def_id);
281+
self.check_where_clauses(fcx, item.span, trait_def_id);
282282
vec![]
283283
});
284284
}
@@ -354,23 +354,6 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
354354
fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
355355
span: Span,
356356
def_id: DefId) {
357-
self.inner_check_where_clauses(fcx, span, def_id, false)
358-
}
359-
360-
fn check_trait_where_clauses<'fcx, 'tcx>(&mut self,
361-
fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
362-
span: Span,
363-
def_id: DefId) {
364-
self.inner_check_where_clauses(fcx, span, def_id, true)
365-
}
366-
367-
/// Checks where clauses and inline bounds that are declared on def_id.
368-
fn inner_check_where_clauses<'fcx, 'tcx>(&mut self,
369-
fcx: &FnCtxt<'fcx, 'gcx, 'tcx>,
370-
span: Span,
371-
def_id: DefId,
372-
is_trait: bool)
373-
{
374357
use ty::subst::Subst;
375358
use rustc::ty::TypeFoldable;
376359

@@ -390,43 +373,50 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
390373
}
391374

392375
// Check that trait predicates are WF when params are substituted by their defaults.
393-
// We don't want to overly constrain the predicates that may be written but we
394-
// want to catch obviously wrong cases such as `struct Foo<T: Copy = String>`
395-
// or cases where defaults don't work together such as:
396-
// `struct Foo<T = i32, U = u8> where T: into<U>`
397-
// Therefore we check if a predicate in which all type params are defaulted
398-
// is WF with those defaults simultaneously substituted.
376+
// We don't want to overly constrain the predicates that may be written but we want to
377+
// catch cases where a default my never be applied such as `struct Foo<T: Copy = String>`.
378+
// Therefore we check if a predicate which contains a single type param
379+
// with a concrete default is WF with that default substituted.
399380
// For more examples see tests `defaults-well-formedness.rs` and `type-check-defaults.rs`.
400381
//
401382
// First we build the defaulted substitution.
402383
let substs = ty::subst::Substs::for_item(fcx.tcx, def_id, |def, _| {
403384
// All regions are identity.
404385
fcx.tcx.mk_region(ty::ReEarlyBound(def.to_early_bound_region_data()))
405386
}, |def, _| {
406-
if !is_our_default(def) {
407-
// We don't want to use non-defaulted params in a substitution, mark as err.
408-
fcx.tcx.types.err
409-
} else {
410-
// Substitute with default.
411-
fcx.tcx.type_of(def.def_id)
387+
// If the param has a default,
388+
if is_our_default(def) {
389+
let default_ty = fcx.tcx.type_of(def.def_id);
390+
// and it's not a dependent default
391+
if !default_ty.needs_subst() {
392+
// then substitute with the default.
393+
return default_ty;
394+
}
412395
}
396+
// Mark unwanted params as err.
397+
fcx.tcx.types.err
413398
});
414399
// Now we build the substituted predicates.
415400
for &pred in predicates.predicates.iter() {
401+
struct CountParams { params: FxHashSet<u32> }
402+
impl<'tcx> ty::fold::TypeVisitor<'tcx> for CountParams {
403+
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
404+
match t.sty {
405+
ty::TyParam(p) => {
406+
self.params.insert(p.idx);
407+
t.super_visit_with(self)
408+
}
409+
_ => t.super_visit_with(self)
410+
}
411+
}
412+
}
413+
let mut param_count = CountParams { params: FxHashSet() };
414+
pred.visit_with(&mut param_count);
416415
let substituted_pred = pred.subst(fcx.tcx, substs);
417-
// If there is a non-defaulted param in the predicate, don't check it.
418-
if substituted_pred.references_error() {
416+
// Don't check non-defaulted params, dependent defaults or preds with multiple params.
417+
if substituted_pred.references_error() || param_count.params.len() > 1 {
419418
continue;
420419
}
421-
// In trait defs, don't check `Self: Sized` when `Self` is the default.
422-
if let ty::Predicate::Trait(trait_pred) = substituted_pred {
423-
// `skip_binder()` is ok, we're only inspecting for `has_self_ty()`.
424-
let lhs_is_self = trait_pred.skip_binder().self_ty().has_self_ty();
425-
let pred_sized = Some(trait_pred.def_id()) == fcx.tcx.lang_items().sized_trait();
426-
if is_trait && lhs_is_self && pred_sized {
427-
continue;
428-
}
429-
}
430420
// Avoid duplication of predicates that contain no parameters, for example.
431421
if !predicates.predicates.contains(&substituted_pred) {
432422
substituted_predicates.push(substituted_pred);

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ struct Foo<U, V=i32>(U, V) where U: Trait<V>;
1414
trait Marker {}
1515
struct TwoParams<T, U>(T, U);
1616
impl Marker for TwoParams<i32, i32> {}
17-
// Check that defaults are substituted simultaneously.
17+
18+
// Clauses with more than 1 param are not checked.
1819
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;
1921
// Clauses with non-defaulted params are not checked.
2022
struct NonDefaultedInClause<T, U = i32>(TwoParams<T, U>) where TwoParams<T, U>: Marker;
2123
struct DefaultedLhs<U, V=i32>(U, V) where V: Trait<U>;
22-
// Dependent defaults.
23-
struct Dependent<T: Copy, U = T>(T, U) where U: Copy;
24+
// Dependent defaults are not checked.
25+
struct Dependent<T, U = T>(T, U) where U: Copy;
26+
trait SelfBound<T: Copy=Self> {}
2427

2528
fn main() {}

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,11 @@ struct WhereClause<T=String>(T) where T: Copy;
3030
trait TraitBound<T:Copy=String> {}
3131
//~^ error: the trait bound `std::string::String: std::marker::Copy` is not satisfied [E0277]
3232

33-
trait SelfBound<T:Copy=Self> {}
34-
//~^ error: the trait bound `Self: std::marker::Copy` is not satisfied [E0277]
35-
3633
trait Super<T: Copy> { }
3734
trait Base<T = String>: Super<T> { }
3835
//~^ error: the trait bound `T: std::marker::Copy` is not satisfied [E0277]
3936

4037
trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
41-
//~^ error: the trait bound `i32: std::ops::Add<u8>` is not satisfied [E0277]
42-
43-
// Defaults must work together.
44-
struct TwoParams<T = u32, U = i32>(T, U) where T: Bar<U>;
45-
//~^ the trait bound `u32: Bar<i32>` is not satisfied [E0277]
46-
trait Bar<V> {}
47-
impl Bar<String> for u32 { }
48-
impl Bar<i32> for String { }
49-
50-
// Dependent defaults.
51-
struct Dependent<T, U = T>(T, U) where U: Copy;
52-
//~^ the trait bound `T: std::marker::Copy` is not satisfied [E0277]
38+
//~^ error: cannot add `u8` to `i32` [E0277]
5339

5440
fn main() { }

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

Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,14 @@ note: required by `Foo`
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2626

2727
error[E0277]: the trait bound `A: std::iter::Iterator` is not satisfied
28-
--> $DIR/type-check-defaults.rs:21:1
28+
--> $DIR/type-check-defaults.rs:21:32
2929
|
3030
21 | struct WellFormedProjection<A, T=<A as Iterator>::Item>(A, T);
31-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `A` is not an iterator; maybe try calling `.iter()` or a similar method
31+
| ^ `A` is not an iterator; maybe try calling `.iter()` or a similar method
3232
|
3333
= help: the trait `std::iter::Iterator` is not implemented for `A`
3434
= help: consider adding a `where A: std::iter::Iterator` bound
35+
= note: required by `std::iter::Iterator`
3536

3637
error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
3738
--> $DIR/type-check-defaults.rs:24:1
@@ -57,59 +58,27 @@ error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not sa
5758
|
5859
= note: required by `std::marker::Copy`
5960

60-
error[E0277]: the trait bound `Self: std::marker::Copy` is not satisfied
61-
--> $DIR/type-check-defaults.rs:33:1
62-
|
63-
33 | trait SelfBound<T:Copy=Self> {}
64-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `Self`
65-
|
66-
= help: consider adding a `where Self: std::marker::Copy` bound
67-
= note: required by `std::marker::Copy`
68-
6961
error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied
70-
--> $DIR/type-check-defaults.rs:37:1
62+
--> $DIR/type-check-defaults.rs:34:1
7163
|
72-
37 | trait Base<T = String>: Super<T> { }
64+
34 | trait Base<T = String>: Super<T> { }
7365
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `T`
7466
|
7567
= help: consider adding a `where T: std::marker::Copy` bound
7668
note: required by `Super`
77-
--> $DIR/type-check-defaults.rs:36:1
69+
--> $DIR/type-check-defaults.rs:33:1
7870
|
79-
36 | trait Super<T: Copy> { }
71+
33 | trait Super<T: Copy> { }
8072
| ^^^^^^^^^^^^^^^^^^^^
8173

82-
error[E0277]: the trait bound `i32: std::ops::Add<u8>` is not satisfied
83-
--> $DIR/type-check-defaults.rs:40:1
74+
error[E0277]: cannot add `u8` to `i32`
75+
--> $DIR/type-check-defaults.rs:37:1
8476
|
85-
40 | trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
77+
37 | trait ProjectionPred<T:Iterator = IntoIter<i32>> where T::Item : Add<u8> {}
8678
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `i32 + u8`
8779
|
8880
= help: the trait `std::ops::Add<u8>` is not implemented for `i32`
8981
= note: required by `std::ops::Add`
9082

91-
error[E0277]: the trait bound `u32: Bar<i32>` is not satisfied
92-
--> $DIR/type-check-defaults.rs:44:1
93-
|
94-
44 | struct TwoParams<T = u32, U = i32>(T, U) where T: Bar<U>;
95-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Bar<i32>` is not implemented for `u32`
96-
|
97-
= help: the following implementations were found:
98-
<u32 as Bar<std::string::String>>
99-
note: required by `Bar`
100-
--> $DIR/type-check-defaults.rs:46:1
101-
|
102-
46 | trait Bar<V> {}
103-
| ^^^^^^^^^^^^
104-
105-
error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied
106-
--> $DIR/type-check-defaults.rs:51:1
107-
|
108-
51 | struct Dependent<T, U = T>(T, U) where U: Copy;
109-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `T`
110-
|
111-
= help: consider adding a `where T: std::marker::Copy` bound
112-
= note: required by `std::marker::Copy`
113-
114-
error: aborting due to 11 previous errors
83+
error: aborting due to 8 previous errors
11584

0 commit comments

Comments
 (0)