Skip to content

Commit

Permalink
Auto merge of #57896 - oli-obk:permissive_existence, r=cramertj
Browse files Browse the repository at this point in the history
 Be more permissive with required bounds on existential types

fixes  #54184

r? @pnkfelix
  • Loading branch information
bors committed Feb 19, 2019
2 parents 4d66b7b + eb98d31 commit 74e35d2
Show file tree
Hide file tree
Showing 39 changed files with 548 additions and 79 deletions.
5 changes: 5 additions & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ impl_stable_hash_for!(struct ty::FnSig<'tcx> {
abi
});

impl_stable_hash_for!(struct ty::ResolvedOpaqueTy<'tcx> {
concrete_type,
substs
});

impl<'a, 'gcx, T> HashStable<StableHashingContext<'a>> for ty::Binder<T>
where T: HashStable<StableHashingContext<'a>>
{
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct OpaqueTypeDecl<'tcx> {
///
/// winds up desugared to:
///
/// abstract type Foo<'x, T>: Trait<'x>
/// abstract type Foo<'x, X>: Trait<'x>
/// fn foo<'a, 'b, T>() -> Foo<'a, T>
///
/// then `substs` would be `['a, T]`.
Expand Down
13 changes: 12 additions & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,17 @@ impl<'a, V> LocalTableInContextMut<'a, V> {
}
}

/// All information necessary to validate and reveal an `impl Trait` or `existential Type`
#[derive(RustcEncodable, RustcDecodable, Debug)]
pub struct ResolvedOpaqueTy<'tcx> {
/// The revealed type as seen by this function.
pub concrete_type: Ty<'tcx>,
/// Generic parameters on the opaque type as passed by this function.
/// For `existential type Foo<A, B>; fn foo<T, U>() -> Foo<T, U> { .. }` this is `[T, U]`, not
/// `[A, B]`
pub substs: &'tcx Substs<'tcx>,
}

#[derive(RustcEncodable, RustcDecodable, Debug)]
pub struct TypeckTables<'tcx> {
/// The HirId::owner all ItemLocalIds in this table are relative to.
Expand Down Expand Up @@ -419,7 +430,7 @@ pub struct TypeckTables<'tcx> {

/// All the existential types that are restricted to concrete types
/// by this function
pub concrete_existential_types: FxHashMap<DefId, Ty<'tcx>>,
pub concrete_existential_types: FxHashMap<DefId, ResolvedOpaqueTy<'tcx>>,

/// Given the closure ID this map provides the list of UpvarIDs used by it.
/// The upvarID contains the HIR node ID and it also contains the full path
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub use self::context::{TyCtxt, FreeRegionInfo, GlobalArenas, AllArenas, tls, ke
pub use self::context::{Lift, TypeckTables, CtxtInterners};
pub use self::context::{
UserTypeAnnotationIndex, UserType, CanonicalUserType,
CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations,
CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, ResolvedOpaqueTy,
};

pub use self::instance::{Instance, InstanceDef};
Expand Down
3 changes: 0 additions & 3 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1386,10 +1386,7 @@ pub fn check_item_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &'tcx hir::Ite
}
hir::ItemKind::Existential(..) => {
let def_id = tcx.hir().local_def_id(it.id);
let pty_ty = tcx.type_of(def_id);
let generics = tcx.generics_of(def_id);

check_bounds_are_used(tcx, &generics, pty_ty);
let substs = Substs::identity_for_item(tcx, def_id);
check_opaque(tcx, def_id, substs, it.span);
}
Expand Down
14 changes: 11 additions & 3 deletions src/librustc_typeck/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,21 +560,29 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
if def_id == defin_ty_def_id {
// Concrete type resolved to the existential type itself
// Force a cycle error
// FIXME(oli-obk): we could just not insert it into `concrete_existential_types`
// which simply would make this use not a defining use.
self.tcx().at(span).type_of(defin_ty_def_id);
}
}

let new = ty::ResolvedOpaqueTy {
concrete_type: definition_ty,
substs: self.tcx().lift_to_global(&opaque_defn.substs).unwrap(),
};

let old = self.tables
.concrete_existential_types
.insert(def_id, definition_ty);
.insert(def_id, new);
if let Some(old) = old {
if old != definition_ty {
if old.concrete_type != definition_ty || old.substs != opaque_defn.substs {
span_bug!(
span,
"visit_opaque_types tried to write \
different types for the same existential type: {:?}, {:?}, {:?}",
different types for the same existential type: {:?}, {:?}, {:?}, {:?}",
def_id,
definition_ty,
opaque_defn,
old,
);
}
Expand Down
112 changes: 101 additions & 11 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ use crate::middle::resolve_lifetime as rl;
use crate::middle::weak_lang_items;
use rustc::mir::mono::Linkage;
use rustc::ty::query::Providers;
use rustc::ty::subst::Substs;
use rustc::ty::subst::{Subst, Substs};
use rustc::ty::util::Discr;
use rustc::ty::util::IntTypeExt;
use rustc::ty::subst::UnpackedKind;
use rustc::ty::{self, AdtKind, ToPolyTraitRef, Ty, TyCtxt};
use rustc::ty::{ReprOptions, ToPredicate};
use rustc::util::captures::Captures;
Expand Down Expand Up @@ -1211,7 +1212,7 @@ fn type_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Ty<'tcx> {
tcx.typeck_tables_of(owner)
.concrete_existential_types
.get(&def_id)
.cloned()
.map(|opaque| opaque.concrete_type)
.unwrap_or_else(|| {
// This can occur if some error in the
// owner fn prevented us from populating
Expand Down Expand Up @@ -1343,7 +1344,13 @@ fn find_existential_constraints<'a, 'tcx>(
struct ConstraintLocator<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId,
found: Option<(Span, ty::Ty<'tcx>)>,
// First found type span, actual type, mapping from the existential type's generic
// parameters to the concrete type's generic parameters
//
// The mapping is an index for each use site of a generic parameter in the concrete type
//
// The indices index into the generic parameters on the existential type.
found: Option<(Span, ty::Ty<'tcx>, Vec<usize>)>,
}

impl<'a, 'tcx> ConstraintLocator<'a, 'tcx> {
Expand All @@ -1358,23 +1365,106 @@ fn find_existential_constraints<'a, 'tcx>(
.tcx
.typeck_tables_of(def_id)
.concrete_existential_types
.get(&self.def_id)
.cloned();
if let Some(ty) = ty {
.get(&self.def_id);
if let Some(ty::ResolvedOpaqueTy { concrete_type, substs }) = ty {
// FIXME(oli-obk): trace the actual span from inference to improve errors
let span = self.tcx.def_span(def_id);
if let Some((prev_span, prev_ty)) = self.found {
if ty != prev_ty {
// used to quickly look up the position of a generic parameter
let mut index_map: FxHashMap<ty::ParamTy, usize> = FxHashMap::default();
// skip binder is ok, since we only use this to find generic parameters and their
// positions.
for (idx, subst) in substs.iter().enumerate() {
if let UnpackedKind::Type(ty) = subst.unpack() {
if let ty::Param(p) = ty.sty {
if index_map.insert(p, idx).is_some() {
// there was already an entry for `p`, meaning a generic parameter
// was used twice
self.tcx.sess.span_err(
span,
&format!("defining existential type use restricts existential \
type by using the generic parameter `{}` twice", p.name),
);
return;
}
} else {
self.tcx.sess.delay_span_bug(
span,
&format!(
"non-defining exist ty use in defining scope: {:?}, {:?}",
concrete_type, substs,
),
);
}
}
}
// compute the index within the existential type for each generic parameter used in
// the concrete type
let indices = concrete_type
.subst(self.tcx, substs)
.walk()
.filter_map(|t| match &t.sty {
ty::Param(p) => Some(*index_map.get(p).unwrap()),
_ => None,
}).collect();
let is_param = |ty: ty::Ty| match ty.sty {
ty::Param(_) => true,
_ => false,
};
if !substs.types().all(is_param) {
self.tcx.sess.span_err(
span,
"defining existential type use does not fully define existential type",
);
} else if let Some((prev_span, prev_ty, ref prev_indices)) = self.found {
let mut ty = concrete_type.walk().fuse();
let mut p_ty = prev_ty.walk().fuse();
let iter_eq = (&mut ty).zip(&mut p_ty).all(|(t, p)| match (&t.sty, &p.sty) {
// type parameters are equal to any other type parameter for the purpose of
// concrete type equality, as it is possible to obtain the same type just
// by passing matching parameters to a function.
(ty::Param(_), ty::Param(_)) => true,
_ => t == p,
});
if !iter_eq || ty.next().is_some() || p_ty.next().is_some() {
// found different concrete types for the existential type
let mut err = self.tcx.sess.struct_span_err(
span,
"defining existential type use differs from previous",
"concrete type differs from previous defining existential type use",
);
err.span_label(
span,
format!("expected `{}`, got `{}`", prev_ty, concrete_type),
);
err.span_note(prev_span, "previous use here");
err.emit();
} else if indices != *prev_indices {
// found "same" concrete types, but the generic parameter order differs
let mut err = self.tcx.sess.struct_span_err(
span,
"concrete type's generic parameters differ from previous defining use",
);
use std::fmt::Write;
let mut s = String::new();
write!(s, "expected [").unwrap();
let list = |s: &mut String, indices: &Vec<usize>| {
let mut indices = indices.iter().cloned();
if let Some(first) = indices.next() {
write!(s, "`{}`", substs[first]).unwrap();
for i in indices {
write!(s, ", `{}`", substs[i]).unwrap();
}
}
};
list(&mut s, prev_indices);
write!(s, "], got [").unwrap();
list(&mut s, &indices);
write!(s, "]").unwrap();
err.span_label(span, s);
err.span_note(prev_span, "previous use here");
err.emit();
}
} else {
self.found = Some((span, ty));
self.found = Some((span, concrete_type, indices));
}
}
}
Expand Down Expand Up @@ -1433,7 +1523,7 @@ fn find_existential_constraints<'a, 'tcx>(
}

match locator.found {
Some((_, ty)) => ty,
Some((_, ty, _)) => ty,
None => {
let span = tcx.def_span(def_id);
tcx.sess.span_err(span, "could not find defining uses");
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/existential_types/bound_reduction2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ trait TraitWithAssoc {
}

existential type Foo<V>: Trait<V>;
//~^ ERROR could not find defining uses

trait Trait<U> {}

impl<W> Trait<W> for () {}

fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> { //~ ERROR non-defining
fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> { //~ ERROR does not fully define
()
}
16 changes: 8 additions & 8 deletions src/test/ui/existential_types/bound_reduction2.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
error: non-defining existential type use in defining scope
--> $DIR/bound_reduction2.rs:16:1
error: defining existential type use does not fully define existential type
--> $DIR/bound_reduction2.rs:17:1
|
LL | / fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> { //~ ERROR non-defining
LL | / fn foo_desugared<T: TraitWithAssoc>(_: T) -> Foo<T::Assoc> { //~ ERROR does not fully define
LL | | ()
LL | | }
| |_^
|
note: used non-generic type <T as TraitWithAssoc>::Assoc for generic parameter
--> $DIR/bound_reduction2.rs:10:22

error: could not find defining uses
--> $DIR/bound_reduction2.rs:10:1
|
LL | existential type Foo<V>: Trait<V>;
| ^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/existential_types/different_defining_uses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ fn foo() -> Foo {
""
}

fn bar() -> Foo { //~ ERROR defining existential type use differs from previous
fn bar() -> Foo { //~ ERROR concrete type differs from previous
42i32
}
6 changes: 3 additions & 3 deletions src/test/ui/existential_types/different_defining_uses.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: defining existential type use differs from previous
error: concrete type differs from previous defining existential type use
--> $DIR/different_defining_uses.rs:12:1
|
LL | / fn bar() -> Foo { //~ ERROR defining existential type use differs from previous
LL | / fn bar() -> Foo { //~ ERROR concrete type differs from previous
LL | | 42i32
LL | | }
| |_^
| |_^ expected `&'static str`, got `i32`
|
note: previous use here
--> $DIR/different_defining_uses.rs:8:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ fn foo() -> Foo {
""
}

fn bar() -> Foo { //~ ERROR defining existential type use differs from previous
fn bar() -> Foo { //~ ERROR concrete type differs from previous
panic!()
}

fn boo() -> Foo { //~ ERROR defining existential type use differs from previous
fn boo() -> Foo { //~ ERROR concrete type differs from previous
loop {}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: defining existential type use differs from previous
error: concrete type differs from previous defining existential type use
--> $DIR/different_defining_uses_never_type.rs:12:1
|
LL | / fn bar() -> Foo { //~ ERROR defining existential type use differs from previous
LL | / fn bar() -> Foo { //~ ERROR concrete type differs from previous
LL | | panic!()
LL | | }
| |_^
| |_^ expected `&'static str`, got `()`
|
note: previous use here
--> $DIR/different_defining_uses_never_type.rs:8:1
Expand All @@ -14,13 +14,13 @@ LL | | ""
LL | | }
| |_^

error: defining existential type use differs from previous
error: concrete type differs from previous defining existential type use
--> $DIR/different_defining_uses_never_type.rs:16:1
|
LL | / fn boo() -> Foo { //~ ERROR defining existential type use differs from previous
LL | / fn boo() -> Foo { //~ ERROR concrete type differs from previous
LL | | loop {}
LL | | }
| |_^
| |_^ expected `&'static str`, got `()`
|
note: previous use here
--> $DIR/different_defining_uses_never_type.rs:8:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ fn my_iter<T>(t: T) -> MyIter<T> {
std::iter::once(t)
}

fn my_iter2<T>(t: T) -> MyIter<T> { //~ ERROR defining existential type use differs from previous
fn my_iter2<T>(t: T) -> MyIter<T> { //~ ERROR concrete type differs from previous
Some(t).into_iter()
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error: defining existential type use differs from previous
error: concrete type differs from previous defining existential type use
--> $DIR/generic_different_defining_uses.rs:11:1
|
LL | / fn my_iter2<T>(t: T) -> MyIter<T> { //~ ERROR defining existential type use differs from previous
LL | / fn my_iter2<T>(t: T) -> MyIter<T> { //~ ERROR concrete type differs from previous
LL | | Some(t).into_iter()
LL | | }
| |_^
| |_^ expected `std::iter::Once<T>`, got `std::option::IntoIter<T>`
|
note: previous use here
--> $DIR/generic_different_defining_uses.rs:7:1
Expand Down
Loading

0 comments on commit 74e35d2

Please sign in to comment.