Skip to content

Commit

Permalink
Addressed more points raised in review.
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexander Regueiro committed May 20, 2019
1 parent 2009662 commit 783b713
Show file tree
Hide file tree
Showing 12 changed files with 452 additions and 185 deletions.
96 changes: 42 additions & 54 deletions src/librustc/traits/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,8 @@ struct PredicateSet<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
}

impl<'a, 'gcx, 'tcx> PredicateSet<'a, 'gcx, 'tcx> {
fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>) -> PredicateSet<'a, 'gcx, 'tcx> {
PredicateSet { tcx: tcx, set: Default::default() }
}

fn contains(&mut self, pred: &ty::Predicate<'tcx>) -> bool {
// See the `insert` method for why we use `anonymize_predicate` here.
self.set.contains(&anonymize_predicate(self.tcx, pred))
fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Self {
Self { tcx: tcx, set: Default::default() }
}

fn insert(&mut self, pred: &ty::Predicate<'tcx>) -> bool {
Expand All @@ -73,11 +68,6 @@ impl<'a, 'gcx, 'tcx> PredicateSet<'a, 'gcx, 'tcx> {
// regions before we throw things into the underlying set.
self.set.insert(anonymize_predicate(self.tcx, pred))
}

fn remove(&mut self, pred: &ty::Predicate<'tcx>) -> bool {
// See the `insert` method for why we use `anonymize_predicate` here.
self.set.remove(&anonymize_predicate(self.tcx, pred))
}
}

impl<'a, 'gcx, 'tcx, T: AsRef<ty::Predicate<'tcx>>> Extend<T> for PredicateSet<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -135,7 +125,7 @@ impl<'cx, 'gcx, 'tcx> Elaborator<'cx, 'gcx, 'tcx> {
FilterToTraits::new(self)
}

fn push(&mut self, predicate: &ty::Predicate<'tcx>) {
fn elaborate(&mut self, predicate: &ty::Predicate<'tcx>) {
let tcx = self.visited.tcx;
match *predicate {
ty::Predicate::Trait(ref data) => {
Expand All @@ -153,7 +143,7 @@ impl<'cx, 'gcx, 'tcx> Elaborator<'cx, 'gcx, 'tcx> {
// This is necessary to prevent infinite recursion in some
// cases. One common case is when people define
// `trait Sized: Sized { }` rather than `trait Sized { }`.
predicates.retain(|p| self.visited.insert(p));
predicates.retain(|pred| self.visited.insert(pred));

self.stack.extend(predicates);
}
Expand Down Expand Up @@ -251,15 +241,12 @@ impl<'cx, 'gcx, 'tcx> Iterator for Elaborator<'cx, 'gcx, 'tcx> {

fn next(&mut self) -> Option<ty::Predicate<'tcx>> {
// Extract next item from top-most stack frame, if any.
let next_predicate = match self.stack.pop() {
Some(predicate) => predicate,
None => {
// No more stack frames. Done.
return None;
}
};
self.push(&next_predicate);
return Some(next_predicate);
if let Some(pred) = self.stack.pop() {
self.elaborate(&pred);
Some(pred)
} else {
None
}
}
}

Expand Down Expand Up @@ -294,31 +281,29 @@ pub fn transitive_bounds<'cx, 'gcx, 'tcx>(tcx: TyCtxt<'cx, 'gcx, 'tcx>,
/// Expansion is done via a DFS (depth-first search), and the `visited` field
/// is used to avoid cycles.
pub struct TraitAliasExpander<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
stack: Vec<TraitAliasExpansionInfo<'tcx>>,
/// The set of predicates visited from the root directly to the current point in the
/// expansion tree (only containing trait aliases).
visited: PredicateSet<'a, 'gcx, 'tcx>,
}

/// Stores information about the expansion of a trait via a path of zero or more trait aliases.
#[derive(Debug, Clone)]
pub struct TraitAliasExpansionInfo<'tcx> {
pub items: SmallVec<[(ty::PolyTraitRef<'tcx>, Span); 4]>,
pub path: SmallVec<[(ty::PolyTraitRef<'tcx>, Span); 4]>,
}

impl<'tcx> TraitAliasExpansionInfo<'tcx> {
fn new(trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
Self {
items: smallvec![(trait_ref, span)]
path: smallvec![(trait_ref, span)]
}
}

fn push(&self, trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
let mut items = self.items.clone();
items.push((trait_ref, span));
fn clone_and_push(&self, trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
let mut path = self.path.clone();
path.push((trait_ref, span));

Self {
items
path
}
}

Expand All @@ -327,11 +312,11 @@ impl<'tcx> TraitAliasExpansionInfo<'tcx> {
}

pub fn top(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
self.items.last().unwrap()
self.path.last().unwrap()
}

pub fn bottom(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
self.items.first().unwrap()
self.path.first().unwrap()
}
}

Expand All @@ -340,21 +325,25 @@ impl<'tcx> TraitAliasExpansionInfo<'tcx> {
pub trait TraitAliasExpansionInfoDignosticBuilder {
fn label_with_exp_info<'tcx>(&mut self,
info: &TraitAliasExpansionInfo<'tcx>,
top_label: &str
top_label: &str,
use_desc: &str
) -> &mut Self;
}

impl<'a> TraitAliasExpansionInfoDignosticBuilder for DiagnosticBuilder<'a> {
fn label_with_exp_info<'tcx>(&mut self,
info: &TraitAliasExpansionInfo<'tcx>,
top_label: &str
top_label: &str,
use_desc: &str
) -> &mut Self {
self.span_label(info.top().1, top_label);
if info.items.len() > 1 {
for (_, sp) in info.items[1..(info.items.len() - 1)].iter().rev() {
self.span_label(*sp, "referenced here");
if info.path.len() > 1 {
for (_, sp) in info.path.iter().rev().skip(1).take(info.path.len() - 2) {
self.span_label(*sp, format!("referenced here ({})", use_desc));
}
}
self.span_label(info.bottom().1,
format!("trait alias used in trait object type ({})", use_desc));
self
}
}
Expand All @@ -367,7 +356,7 @@ pub fn expand_trait_aliases<'cx, 'gcx, 'tcx>(
.into_iter()
.map(|(trait_ref, span)| TraitAliasExpansionInfo::new(trait_ref, span))
.collect();
TraitAliasExpander { stack: items, visited: PredicateSet::new(tcx) }
TraitAliasExpander { tcx, stack: items }
}

impl<'cx, 'gcx, 'tcx> TraitAliasExpander<'cx, 'gcx, 'tcx> {
Expand All @@ -376,23 +365,25 @@ impl<'cx, 'gcx, 'tcx> TraitAliasExpander<'cx, 'gcx, 'tcx> {
/// Otherwise, immediately returns `true` if `item` is a regular trait, or `false` if it is a
/// trait alias.
/// The return value indicates whether `item` should be yielded to the user.
fn push(&mut self, item: &TraitAliasExpansionInfo<'tcx>) -> bool {
let tcx = self.visited.tcx;
fn expand(&mut self, item: &TraitAliasExpansionInfo<'tcx>) -> bool {
let tcx = self.tcx;
let trait_ref = item.trait_ref();
let pred = trait_ref.to_predicate();

debug!("expand_trait_aliases: trait_ref={:?}", trait_ref);

// Don't recurse unless this bound is a trait alias and isn't currently in the DFS stack of
// already-visited predicates.
// Don't recurse if this bound is not a trait alias.
let is_alias = tcx.is_trait_alias(trait_ref.def_id());
if !is_alias || self.visited.contains(&pred) {
return !is_alias;
if !is_alias {
return true;
}

// Remove the current predicate from the stack of already-visited ones, since we're doing
// a DFS.
self.visited.remove(&pred);
// Don't recurse if this trait alias is already on the stack for the DFS search.
let anon_pred = anonymize_predicate(tcx, &pred);
if item.path.iter().rev().skip(1)
.any(|(tr, _)| anonymize_predicate(tcx, &tr.to_predicate()) == anon_pred) {
return false;
}

// Get components of trait alias.
let predicates = tcx.super_predicates_of(trait_ref.def_id());
Expand All @@ -403,16 +394,13 @@ impl<'cx, 'gcx, 'tcx> TraitAliasExpander<'cx, 'gcx, 'tcx> {
.filter_map(|(pred, span)| {
pred.subst_supertrait(tcx, &trait_ref)
.to_opt_poly_trait_ref()
.map(|trait_ref| item.push(trait_ref, *span))
.map(|trait_ref| item.clone_and_push(trait_ref, *span))
})
.collect();
debug!("expand_trait_aliases: items={:?}", items);

self.stack.extend(items);

// Record predicate into set of already-visited.
self.visited.insert(&pred);

false
}
}
Expand All @@ -426,7 +414,7 @@ impl<'cx, 'gcx, 'tcx> Iterator for TraitAliasExpander<'cx, 'gcx, 'tcx> {

fn next(&mut self) -> Option<TraitAliasExpansionInfo<'tcx>> {
while let Some(item) = self.stack.pop() {
if self.push(&item) {
if self.expand(&item) {
return Some(item);
}
}
Expand Down
30 changes: 17 additions & 13 deletions src/librustc_typeck/astconv.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,14 +648,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
// careful!
if default_needs_object_self(param) {
struct_span_err!(tcx.sess, span, E0393,
"the type parameter `{}` must be explicitly \
specified",
param.name)
.span_label(span,
format!("missing reference to `{}`", param.name))
.note(&format!("because of the default `Self` reference, \
type parameters must be specified on object \
types"))
"the type parameter `{}` must be explicitly specified",
param.name
)
.span_label(span, format!(
"missing reference to `{}`", param.name))
.note(&format!(
"because of the default `Self` reference, type parameters \
must be specified on object types"))
.emit();
tcx.types.err.into()
} else {
Expand Down Expand Up @@ -987,20 +987,24 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
);
potential_assoc_types.extend(cur_potential_assoc_types.into_iter().flatten());
(trait_ref, trait_bound.span)
}).collect();
})
.collect();

// Expand trait aliases recursively and check that only one regular (non-auto) trait
// is used and no 'maybe' bounds are used.
let expanded_traits = traits::expand_trait_aliases(tcx, bound_trait_refs.clone());
let (mut auto_traits, regular_traits): (Vec<_>, Vec<_>) =
expanded_traits.partition(|i| tcx.trait_is_auto(i.trait_ref().def_id()));
if regular_traits.len() > 1 {
let extra_trait = &regular_traits[1];
struct_span_err!(tcx.sess, extra_trait.bottom().1, E0225,
let first_trait = &regular_traits[0];
let additional_trait = &regular_traits[1];
struct_span_err!(tcx.sess, additional_trait.bottom().1, E0225,
"only auto traits can be used as additional traits in a trait object"
)
.label_with_exp_info(extra_trait, "additional non-auto trait")
.span_label(regular_traits[0].top().1, "first non-auto trait")
.label_with_exp_info(additional_trait, "additional non-auto trait",
"additional use")
.label_with_exp_info(first_trait, "first non-auto trait",
"first use")
.emit();
}

Expand Down
7 changes: 5 additions & 2 deletions src/test/ui/bad/bad-sized.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ error[E0225]: only auto traits can be used as additional traits in a trait objec
--> $DIR/bad-sized.rs:4:24
|
LL | let x: Vec<Trait + Sized> = Vec::new();
| ----- ^^^^^ additional non-auto trait
| |
| ----- ^^^^^
| | |
| | additional non-auto trait
| | trait alias used in trait object type (additional use)
| first non-auto trait
| trait alias used in trait object type (first use)

error[E0277]: the size for values of type `dyn Trait` cannot be known at compilation time
--> $DIR/bad-sized.rs:4:12
Expand Down
22 changes: 14 additions & 8 deletions src/test/ui/error-codes/E0225.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
error[E0225]: only auto traits can be used as additional traits in a trait object
--> $DIR/E0225.rs:6:32
--> $DIR/E0225.rs:6:36
|
LL | let _: Box<std::io::Read + std::io::Write>;
| ------------- ^^^^^^^^^^^^^^ additional non-auto trait
| |
| first non-auto trait
LL | let _: Box<dyn std::io::Read + std::io::Write>;
| ------------- ^^^^^^^^^^^^^^
| | |
| | additional non-auto trait
| | trait alias used in trait object type (additional use)
| first non-auto trait
| trait alias used in trait object type (first use)

error[E0225]: only auto traits can be used as additional traits in a trait object
--> $DIR/E0225.rs:8:16
--> $DIR/E0225.rs:8:20
|
LL | trait Foo = std::io::Read + std::io::Write;
| ------------- -------------- additional non-auto trait
| |
| first non-auto trait
...
LL | let _: Box<Foo>;
| ^^^
LL | let _: Box<dyn Foo>;
| ^^^
| |
| trait alias used in trait object type (additional use)
| trait alias used in trait object type (first use)

error: aborting due to 2 previous errors

Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-22560.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@ error[E0225]: only auto traits can be used as additional traits in a trait objec
--> $DIR/issue-22560.rs:6:13
|
LL | type Test = Add +
| --- first non-auto trait
| ---
| |
| first non-auto trait
| trait alias used in trait object type (first use)
...
LL | Sub;
| ^^^ additional non-auto trait
| ^^^
| |
| additional non-auto trait
| trait alias used in trait object type (additional use)

error[E0191]: the value of the associated types `Output` (from the trait `std::ops::Add`), `Output` (from the trait `std::ops::Sub`) must be specified
--> $DIR/issue-22560.rs:3:13
Expand Down
7 changes: 5 additions & 2 deletions src/test/ui/issues/issue-32963.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ error[E0225]: only auto traits can be used as additional traits in a trait objec
--> $DIR/issue-32963.rs:8:25
|
LL | size_of_copy::<Misc+Copy>();
| ---- ^^^^ additional non-auto trait
| |
| ---- ^^^^
| | |
| | additional non-auto trait
| | trait alias used in trait object type (additional use)
| first non-auto trait
| trait alias used in trait object type (first use)

error[E0277]: the trait bound `dyn Misc: std::marker::Copy` is not satisfied
--> $DIR/issue-32963.rs:8:5
Expand Down
Loading

0 comments on commit 783b713

Please sign in to comment.