Skip to content

Remove duplicate predicates in explicit_predicates_of #54780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1637,9 +1637,39 @@ fn explicit_predicates_of<'a, 'tcx>(
def_id: DefId,
) -> ty::GenericPredicates<'tcx> {
use rustc::hir::*;
use rustc_data_structures::fx::FxHashSet;

debug!("explicit_predicates_of(def_id={:?})", def_id);

/// A data structure with unique elements, which preserves order of insertion.
/// Preserving the order of insertion is important here so as not to break
/// compile-fail UI tests.
struct UniquePredicates<'tcx> {
predicates: Vec<(ty::Predicate<'tcx>, Span)>,
uniques: FxHashSet<(ty::Predicate<'tcx>, Span)>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if we should add the indexmap crate and use that instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meh, leave it for now

}

impl<'tcx> UniquePredicates<'tcx> {
fn new() -> Self {
UniquePredicates {
predicates: vec![],
uniques: FxHashSet::default(),
}
}

fn push(&mut self, value: (ty::Predicate<'tcx>, Span)) {
if self.uniques.insert(value) {
self.predicates.push(value);
Copy link
Contributor

@ljedrz ljedrz Oct 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do predicates need to contain copies of the elements in uniques? Since both have the same elements and the Vec is used only (as far as I understand) to keep the order, maybe it could hold just references to the elements in uniques instead? On the other hand, Predicate is Copy...

Copy link
Member Author

@scalexm scalexm Oct 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes everything is Copy (these are references to arena-allocated memory) so I didn’t care to do such an optimization :)

}
}

fn extend<I: IntoIterator<Item = (ty::Predicate<'tcx>, Span)>>(&mut self, iter: I) {
for value in iter {
self.push(value);
}
}
}

let node_id = tcx.hir.as_local_node_id(def_id).unwrap();
let node = tcx.hir.get(node_id);

Expand All @@ -1649,7 +1679,7 @@ fn explicit_predicates_of<'a, 'tcx>(
let icx = ItemCtxt::new(tcx, def_id);
let no_generics = hir::Generics::empty();

let mut predicates = vec![];
let mut predicates = UniquePredicates::new();

let ast_generics = match node {
Node::TraitItem(item) => &item.generics,
Expand Down Expand Up @@ -1744,7 +1774,7 @@ fn explicit_predicates_of<'a, 'tcx>(
// on a trait we need to add in the supertrait bounds and bounds found on
// associated types.
if let Some((_trait_ref, _)) = is_trait {
predicates = tcx.super_predicates_of(def_id).predicates;
predicates.extend(tcx.super_predicates_of(def_id).predicates);
}

// In default impls, we can assume that the self type implements
Expand Down Expand Up @@ -1895,6 +1925,8 @@ fn explicit_predicates_of<'a, 'tcx>(
}))
}

let mut predicates = predicates.predicates;

// Subtle: before we store the predicates into the tcx, we
// sort them so that predicates like `T: Foo<Item=U>` come
// before uses of `U`. This avoids false ambiguity errors
Expand Down
6 changes: 2 additions & 4 deletions src/test/ui/chalkify/lower_env1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ LL | #[rustc_dump_program_clauses] //~ ERROR program clause dump
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: FromEnv(Self: Foo) :- FromEnv(Self: Bar).
= note: FromEnv(Self: Foo) :- FromEnv(Self: Bar).
= note: Implemented(Self: Bar) :- FromEnv(Self: Bar).
= note: WellFormed(Self: Bar) :- Implemented(Self: Bar), WellFormed(Self: Foo), WellFormed(Self: Foo).
= note: WellFormed(Self: Bar) :- Implemented(Self: Bar), WellFormed(Self: Foo).

error: program clause dump
--> $DIR/lower_env1.rs:19:1
Expand All @@ -16,11 +15,10 @@ LL | #[rustc_dump_env_program_clauses] //~ ERROR program clause dump
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: FromEnv(Self: Foo) :- FromEnv(Self: Bar).
= note: FromEnv(Self: Foo) :- FromEnv(Self: Bar).
= note: Implemented(Self: Bar) :- FromEnv(Self: Bar).
= note: Implemented(Self: Foo) :- FromEnv(Self: Foo).
= note: Implemented(Self: std::marker::Sized) :- FromEnv(Self: std::marker::Sized).
= note: WellFormed(Self: Bar) :- Implemented(Self: Bar), WellFormed(Self: Foo), WellFormed(Self: Foo).
= note: WellFormed(Self: Bar) :- Implemented(Self: Bar), WellFormed(Self: Foo).
= note: WellFormed(Self: Foo) :- Implemented(Self: Foo).
= note: WellFormed(Self: std::marker::Sized) :- Implemented(Self: std::marker::Sized).

Expand Down