Skip to content

Commit

Permalink
typeck: merge opaque type inference logic
Browse files Browse the repository at this point in the history
This commit merges the logic used for opaque type type inference for
impl Trait and non-impl Trait cases. This fixes an ICE where
existential types used in the return types of functions would be allowed
to have an out-of-scope generic type parameter.
  • Loading branch information
davidtwco committed Jul 3, 2019
1 parent 8301de1 commit de8660a
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 139 deletions.
76 changes: 75 additions & 1 deletion src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::hir::Node;
use crate::infer::outlives::free_region_map::FreeRegionRelations;
use crate::infer::{self, InferCtxt, InferOk, TypeVariableOrigin, TypeVariableOriginKind};
use crate::middle::region;
use crate::mir::interpret::ConstValue;
use crate::traits::{self, PredicateObligation};
use crate::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder, TypeVisitor};
use crate::ty::subst::{InternalSubsts, Kind, SubstsRef, UnpackedKind};
Expand Down Expand Up @@ -553,6 +554,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
def_id: DefId,
opaque_defn: &OpaqueTypeDecl<'tcx>,
instantiated_ty: Ty<'tcx>,
span: Span,
) -> Ty<'tcx> {
debug!(
"infer_opaque_definition_from_instantiation(def_id={:?}, instantiated_ty={:?})",
Expand Down Expand Up @@ -584,6 +586,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
def_id,
map,
instantiated_ty,
span,
));
debug!("infer_opaque_definition_from_instantiation: definition_ty={:?}", definition_ty);

Expand Down Expand Up @@ -761,6 +764,9 @@ struct ReverseMapper<'tcx> {

/// initially `Some`, set to `None` once error has been reported
hidden_ty: Option<Ty<'tcx>>,

/// Span of function being checked.
span: Span,
}

impl ReverseMapper<'tcx> {
Expand All @@ -770,6 +776,7 @@ impl ReverseMapper<'tcx> {
opaque_type_def_id: DefId,
map: FxHashMap<Kind<'tcx>, Kind<'tcx>>,
hidden_ty: Ty<'tcx>,
span: Span,
) -> Self {
Self {
tcx,
Expand All @@ -778,6 +785,7 @@ impl ReverseMapper<'tcx> {
map,
map_missing_regions_to_empty: false,
hidden_ty: Some(hidden_ty),
span,
}
}

Expand Down Expand Up @@ -812,10 +820,11 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
_ => { }
}

let generics = self.tcx().generics_of(self.opaque_type_def_id);
match self.map.get(&r.into()).map(|k| k.unpack()) {
Some(UnpackedKind::Lifetime(r1)) => r1,
Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
None => {
None if generics.parent.is_some() => {
if !self.map_missing_regions_to_empty && !self.tainted_by_errors {
if let Some(hidden_ty) = self.hidden_ty.take() {
unexpected_hidden_region_diagnostic(
Expand All @@ -829,6 +838,21 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
}
self.tcx.lifetimes.re_empty
}
None => {
self.tcx.sess
.struct_span_err(
self.span,
"non-defining existential type use in defining scope"
)
.span_label(
self.span,
format!("lifetime `{}` is part of concrete type but not used in \
parameter list of existential type", r),
)
.emit();

self.tcx().global_tcx().mk_region(ty::ReStatic)
},
}
}

Expand Down Expand Up @@ -890,9 +914,59 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
self.tcx.mk_generator(def_id, ty::GeneratorSubsts { substs }, movability)
}

ty::Param(..) => {
// Look it up in the substitution list.
match self.map.get(&ty.into()).map(|k| k.unpack()) {
// Found it in the substitution list; replace with the parameter from the
// existential type.
Some(UnpackedKind::Type(t1)) => t1,
Some(u) => panic!("type mapped to unexpected kind: {:?}", u),
None => {
self.tcx.sess
.struct_span_err(
self.span,
&format!("type parameter `{}` is part of concrete type but not \
used in parameter list for existential type", ty),
)
.emit();

self.tcx().types.err
}
}
}

_ => ty.super_fold_with(self),
}
}

fn fold_const(&mut self, ct: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> {
trace!("checking const {:?}", ct);
// Find a const parameter
match ct.val {
ConstValue::Param(..) => {
// Look it up in the substitution list.
match self.map.get(&ct.into()).map(|k| k.unpack()) {
// Found it in the substitution list, replace with the parameter from the
// existential type.
Some(UnpackedKind::Const(c1)) => c1,
Some(u) => panic!("const mapped to unexpected kind: {:?}", u),
None => {
self.tcx.sess
.struct_span_err(
self.span,
&format!("const parameter `{}` is part of concrete type but not \
used in parameter list for existential type", ct)
)
.emit();

self.tcx().consts.err
}
}
}

_ => ct,
}
}
}

struct Instantiator<'a, 'tcx> {
Expand Down
153 changes: 15 additions & 138 deletions src/librustc_typeck/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ use rustc::hir::def_id::{DefId, DefIndex};
use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc::infer::InferCtxt;
use rustc::ty::adjustment::{Adjust, Adjustment, PointerCast};
use rustc::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder};
use rustc::ty::subst::UnpackedKind;
use rustc::ty::fold::{TypeFoldable, TypeFolder};
use rustc::ty::{self, Ty, TyCtxt};
use rustc::mir::interpret::ConstValue;
use rustc::util::nodemap::DefIdSet;
use rustc_data_structures::sync::Lrc;
use std::mem;
Expand Down Expand Up @@ -440,141 +438,20 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {

debug_assert!(!instantiated_ty.has_escaping_bound_vars());

let generics = self.tcx().generics_of(def_id);

let definition_ty = if generics.parent.is_some() {
// `impl Trait`
self.fcx.infer_opaque_definition_from_instantiation(
def_id,
opaque_defn,
instantiated_ty,
)
} else {
// Prevent:
// * `fn foo<T>() -> Foo<T>`
// * `fn foo<T: Bound + Other>() -> Foo<T>`
// from being defining.

// Also replace all generic params with the ones from the existential type
// definition so that
// ```rust
// existential type Foo<T>: 'static;
// fn foo<U>() -> Foo<U> { .. }
// ```
// figures out the concrete type with `U`, but the stored type is with `T`.
instantiated_ty.fold_with(&mut BottomUpFolder {
tcx: self.tcx().global_tcx(),
ty_op: |ty| {
trace!("checking type {:?}", ty);
// Find a type parameter.
if let ty::Param(..) = ty.sty {
// Look it up in the substitution list.
assert_eq!(opaque_defn.substs.len(), generics.params.len());
for (subst, param) in opaque_defn.substs.iter().zip(&generics.params) {
if let UnpackedKind::Type(subst) = subst.unpack() {
if subst == ty {
// Found it in the substitution list; replace with the
// parameter from the existential type.
return self.tcx()
.global_tcx()
.mk_ty_param(param.index, param.name);
}
}
}
self.tcx()
.sess
.struct_span_err(
span,
&format!(
"type parameter `{}` is part of concrete type but not used \
in parameter list for existential type",
ty,
),
)
.emit();
return self.tcx().types.err;
}
ty
},
lt_op: |region| {
match region {
// Skip static and bound regions: they don't require substitution.
ty::ReStatic | ty::ReLateBound(..) => region,
_ => {
trace!("checking {:?}", region);
for (subst, p) in opaque_defn.substs.iter().zip(&generics.params) {
if let UnpackedKind::Lifetime(subst) = subst.unpack() {
if subst == region {
// Found it in the substitution list; replace with the
// parameter from the existential type.
let reg = ty::EarlyBoundRegion {
def_id: p.def_id,
index: p.index,
name: p.name,
};
trace!("replace {:?} with {:?}", region, reg);
return self.tcx()
.global_tcx()
.mk_region(ty::ReEarlyBound(reg));
}
}
}
trace!("opaque_defn: {:#?}", opaque_defn);
trace!("generics: {:#?}", generics);
self.tcx()
.sess
.struct_span_err(
span,
"non-defining existential type use in defining scope",
)
.span_label(
span,
format!(
"lifetime `{}` is part of concrete type but not used \
in parameter list of existential type",
region,
),
)
.emit();
self.tcx().global_tcx().mk_region(ty::ReStatic)
}
}
},
ct_op: |ct| {
trace!("checking const {:?}", ct);
// Find a const parameter
if let ConstValue::Param(..) = ct.val {
// look it up in the substitution list
assert_eq!(opaque_defn.substs.len(), generics.params.len());
for (subst, param) in opaque_defn.substs.iter()
.zip(&generics.params) {
if let UnpackedKind::Const(subst) = subst.unpack() {
if subst == ct {
// found it in the substitution list, replace with the
// parameter from the existential type
return self.tcx()
.global_tcx()
.mk_const_param(param.index, param.name, ct.ty);
}
}
}
self.tcx()
.sess
.struct_span_err(
span,
&format!(
"const parameter `{}` is part of concrete type but not \
used in parameter list for existential type",
ct,
),
)
.emit();
return self.tcx().consts.err;
}
ct
}
})
};
// Prevent:
// * `fn foo<T>() -> Foo<T>`
// * `fn foo<T: Bound + Other>() -> Foo<T>`
// from being defining.

// Also replace all generic params with the ones from the existential type
// definition so that
// ```rust
// existential type Foo<T>: 'static;
// fn foo<U>() -> Foo<U> { .. }
// ```
// figures out the concrete type with `U`, but the stored type is with `T`.
let definition_ty = self.fcx.infer_opaque_definition_from_instantiation(
def_id, opaque_defn, instantiated_ty, span);

if let ty::Opaque(defin_ty_def_id, _substs) = definition_ty.sty {
if def_id == defin_ty_def_id {
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/impl-trait/issue-55872-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// ignore-tidy-linelength
#![feature(existential_type)]

pub trait Bar
{
type E: Copy;

fn foo<T>() -> Self::E;
}

impl<S: Default> Bar for S {
existential type E: Copy;
//~^ ERROR the trait bound `S: std::marker::Copy` is not satisfied in `(S, T)` [E0277]
//~^^ ERROR the trait bound `T: std::marker::Copy` is not satisfied in `(S, T)` [E0277]

fn foo<T: Default>() -> Self::E {
//~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for existential type
(S::default(), T::default())
}
}

fn main() {}
33 changes: 33 additions & 0 deletions src/test/ui/impl-trait/issue-55872-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error[E0277]: the trait bound `S: std::marker::Copy` is not satisfied in `(S, T)`
--> $DIR/issue-55872-1.rs:12:5
|
LL | existential type E: Copy;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ within `(S, T)`, the trait `std::marker::Copy` is not implemented for `S`
|
= help: consider adding a `where S: std::marker::Copy` bound
= note: required because it appears within the type `(S, T)`
= note: the return type of a function must have a statically known size

error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied in `(S, T)`
--> $DIR/issue-55872-1.rs:12:5
|
LL | existential type E: Copy;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ within `(S, T)`, the trait `std::marker::Copy` is not implemented for `T`
|
= help: consider adding a `where T: std::marker::Copy` bound
= note: required because it appears within the type `(S, T)`
= note: the return type of a function must have a statically known size

error: type parameter `T` is part of concrete type but not used in parameter list for existential type
--> $DIR/issue-55872-1.rs:16:37
|
LL | fn foo<T: Default>() -> Self::E {
| _____________________________________^
LL | |
LL | | (S::default(), T::default())
LL | | }
| |_____^

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.
20 changes: 20 additions & 0 deletions src/test/ui/impl-trait/issue-55872-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// edition:2018
// ignore-tidy-linelength
#![feature(async_await, existential_type)]

pub trait Bar {
type E: Copy;

fn foo<T>() -> Self::E;
}

impl<S> Bar for S {
existential type E: Copy;
//~^ ERROR the trait bound `impl std::future::Future: std::marker::Copy` is not satisfied [E0277]
fn foo<T>() -> Self::E {
//~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for existential type
async {}
}
}

fn main() {}
Loading

0 comments on commit de8660a

Please sign in to comment.