Skip to content
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

correctly dedup ExistentialPredicates #73815

Closed
wants to merge 4 commits into from
Closed
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
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2419,7 +2419,8 @@ impl<'tcx> TyCtxt<'tcx> {
eps: &[ExistentialPredicate<'tcx>],
) -> &'tcx List<ExistentialPredicate<'tcx>> {
assert!(!eps.is_empty());
assert!(eps.windows(2).all(|w| w[0].stable_cmp(self, &w[1]) != Ordering::Greater));
// Do not allow duplicate existential predicates.
assert!(eps.windows(2).all(|w| w[0].stable_cmp(self, w[1]) == Ordering::Less));
self._intern_existential_predicates(eps)
}

Expand Down
13 changes: 2 additions & 11 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,20 +598,11 @@ impl<'tcx> Relate<'tcx> for &'tcx ty::List<ty::ExistentialPredicate<'tcx>> {
) -> RelateResult<'tcx, Self> {
let tcx = relation.tcx();

// FIXME: this is wasteful, but want to do a perf run to see how slow it is.
// We need to perform this deduplication as we sometimes generate duplicate projections
// in `a`.
let mut a_v: Vec<_> = a.into_iter().collect();
let mut b_v: Vec<_> = b.into_iter().collect();
a_v.sort_by(|a, b| a.stable_cmp(tcx, b));
a_v.dedup();
b_v.sort_by(|a, b| a.stable_cmp(tcx, b));
b_v.dedup();
if a_v.len() != b_v.len() {
if a.len() != b.len() {
return Err(TypeError::ExistentialMismatch(expected_found(relation, a, b)));
}

let v = a_v.into_iter().zip(b_v.into_iter()).map(|(ep_a, ep_b)| {
let v = a.into_iter().zip(b.into_iter()).map(|(ep_a, ep_b)| {
use crate::ty::ExistentialPredicate::*;
match (ep_a, ep_b) {
(Trait(a), Trait(b)) => Ok(Trait(relation.relate(a, b)?)),
Expand Down
32 changes: 26 additions & 6 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_hir::def_id::DefId;
use rustc_index::vec::Idx;
use rustc_macros::HashStable;
use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::DUMMY_SP;
use rustc_target::abi::VariantIdx;
use rustc_target::spec::abi;
use std::borrow::Cow;
Expand Down Expand Up @@ -672,15 +673,34 @@ pub enum ExistentialPredicate<'tcx> {
impl<'tcx> ExistentialPredicate<'tcx> {
/// Compares via an ordering that will not change if modules are reordered or other changes are
/// made to the tree. In particular, this ordering is preserved across incremental compilations.
pub fn stable_cmp(&self, tcx: TyCtxt<'tcx>, other: &Self) -> Ordering {
pub fn stable_cmp(self, tcx: TyCtxt<'tcx>, other: Self) -> Ordering {
use self::ExistentialPredicate::*;
match (*self, *other) {
(Trait(_), Trait(_)) => Ordering::Equal,
(Projection(ref a), Projection(ref b)) => {
// Note that we only call this method after checking that the
// given predicates represent a valid trait object.
//
// This means that we have at most one `ExistentialPredicate::Trait`
// and at most one `ExistentialPredicate::Projection` for each associated item.
match (self, other) {
(Trait(a), Trait(b)) => {
if a != b {
tcx.sess.delay_span_bug(
DUMMY_SP,
&format!("unexpected existential predicates: {:?}, {:?}", a, b),
);
}
Ordering::Equal
}
(Projection(a), Projection(b)) => {
if a.item_def_id == b.item_def_id && a != b {
tcx.sess.delay_span_bug(
DUMMY_SP,
&format!("unexpected existential predicates: {:?}, {:?}", a, b),
);
}
tcx.def_path_hash(a.item_def_id).cmp(&tcx.def_path_hash(b.item_def_id))
}
(AutoTrait(ref a), AutoTrait(ref b)) => {
tcx.trait_def(*a).def_path_hash.cmp(&tcx.trait_def(*b).def_path_hash)
(AutoTrait(a), AutoTrait(b)) => {
tcx.trait_def(a).def_path_hash.cmp(&tcx.trait_def(b).def_path_hash)
}
(Trait(_), _) => Ordering::Less,
(Projection(_), Trait(_)) => Ordering::Greater,
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use rustc_trait_selection::traits::error_reporting::report_object_safety_error;
use rustc_trait_selection::traits::wf::object_region_bounds;

use smallvec::SmallVec;
use std::cmp::Ordering;
use std::collections::BTreeSet;
use std::iter;
use std::slice;
Expand Down Expand Up @@ -1191,8 +1192,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
.map(|x| ty::ExistentialPredicate::Projection(x.skip_binder())),
)
.collect::<SmallVec<[_; 8]>>();
v.sort_by(|a, b| a.stable_cmp(tcx, b));
v.dedup();
v.sort_by(|&a, &b| a.stable_cmp(tcx, b));
v.dedup_by(|&mut a, &mut b| a.stable_cmp(tcx, b) == Ordering::Equal);
let existential_predicates = ty::Binder::bind(tcx.mk_existential_predicates(v.into_iter()));

// Use explicitly-specified region bound.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// check-pass
#![feature(trait_alias)]

trait I32Iterator = Iterator<Item = i32>;

fn main() {
let _: &dyn I32Iterator<Item = u32> = &vec![42].into_iter();
//~^ ERROR type mismatch
Copy link
Contributor Author

@lcnr lcnr Jun 27, 2020

Choose a reason for hiding this comment

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

I think this test changes because ExistentialPredicate::stable_cmp doesn't actually look at its substs. This seems kind of incorrect to me 🤔

}

This file was deleted.

28 changes: 28 additions & 0 deletions src/test/ui/issues/issue-59326-compile-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
trait Service {
type S;
}

trait Framing {
type F;
}

impl Framing for () {
type F = ();
}

impl Framing for u32 {
type F = u32;
}

trait HttpService<F: Framing>: Service<S = F::F> {}

fn build_server<F: FnOnce() -> Box<dyn HttpService<u32, S = ()>>>(_: F) {}

fn make_server<F: Framing>() -> Box<dyn HttpService<F, S = F::F>> {
unimplemented!()
}

fn main() {
build_server(|| make_server())
//~^ ERROR type mismatch
}
9 changes: 9 additions & 0 deletions src/test/ui/issues/issue-59326-compile-fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0271]: type mismatch resolving `<u32 as Framing>::F == ()`
--> $DIR/issue-59326-compile-fail.rs:26:21
|
LL | build_server(|| make_server())
| ^^^^^^^^^^^^^ expected `u32`, found `()`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0271`.