1
Fork 0

Address review comments

This commit is contained in:
Matthew Jasper 2019-12-29 14:23:20 +00:00
parent 93ac5bc7de
commit f23bca79d4
6 changed files with 64 additions and 52 deletions

View file

@ -500,9 +500,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
hir::OpaqueTyOrigin::AsyncFn => return false, hir::OpaqueTyOrigin::AsyncFn => return false,
// Otherwise, generate the label we'll use in the error message. // Otherwise, generate the label we'll use in the error message.
hir::OpaqueTyOrigin::TypeAlias => "impl Trait", hir::OpaqueTyOrigin::TypeAlias
hir::OpaqueTyOrigin::FnReturn => "impl Trait", | hir::OpaqueTyOrigin::FnReturn
hir::OpaqueTyOrigin::Misc => "impl Trait", | hir::OpaqueTyOrigin::Misc => "impl Trait",
}; };
let msg = format!("ambiguous lifetime bound in `{}`", context_name); let msg = format!("ambiguous lifetime bound in `{}`", context_name);
let mut err = self.tcx.sess.struct_span_err(span, &msg); let mut err = self.tcx.sess.struct_span_err(span, &msg);
@ -814,18 +814,29 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> { fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
match r { match r {
// Ignore bound regions that appear in the type, they don't need to // Ignore bound regions and `'static` regions that appear in the
// be remapped (e.g., this would ignore `'r` in a type like // type, we only need to remap regions that reference lifetimes
// `for<'r> fn(&'r u32)`. // from the function declaraion.
ty::ReLateBound(..) // This would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
ty::ReLateBound(..) | ty::ReStatic => return r,
// If regions have been erased, don't try to unerase them. // If regions have been erased (by writeback), don't try to unerase
| ty::ReErased // them.
ty::ReErased => return r,
// ignore `'static`, as that can appear anywhere // The regions that we expect from borrow checking.
| ty::ReStatic => return r, ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReEmpty(ty::UniverseIndex::ROOT) => {}
_ => {} ty::ReEmpty(_)
| ty::RePlaceholder(_)
| ty::ReVar(_)
| ty::ReScope(_)
| ty::ReClosureBound(_) => {
// All of the regions in the type should either have been
// erased by writeback, or mapped back to named regions by
// borrow checking.
bug!("unexpected region kind in opaque type: {:?}", r);
}
} }
let generics = self.tcx().generics_of(self.opaque_type_def_id); let generics = self.tcx().generics_of(self.opaque_type_def_id);
@ -833,7 +844,7 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
Some(GenericArgKind::Lifetime(r1)) => r1, Some(GenericArgKind::Lifetime(r1)) => r1,
Some(u) => panic!("region mapped to unexpected kind: {:?}", u), Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
None if self.map_missing_regions_to_empty || self.tainted_by_errors => { None if self.map_missing_regions_to_empty || self.tainted_by_errors => {
self.tcx.lifetimes.re_empty self.tcx.lifetimes.re_root_empty
} }
None if generics.parent.is_some() => { None if generics.parent.is_some() => {
if let Some(hidden_ty) = self.hidden_ty.take() { if let Some(hidden_ty) = self.hidden_ty.take() {
@ -862,7 +873,7 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
) )
.emit(); .emit();
self.tcx().mk_region(ty::ReStatic) self.tcx().lifetimes.re_static
} }
} }
} }

View file

@ -1723,17 +1723,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) )
} else { } else {
match decl.output { match decl.output {
FunctionRetTy::Ty(ref ty) => match in_band_ty_params { FunctionRetTy::Ty(ref ty) => {
Some((def_id, _)) if impl_trait_return_allow => { let context = match in_band_ty_params {
hir::FunctionRetTy::Return(self.lower_ty( Some((def_id, _)) if impl_trait_return_allow => {
ty, ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn)
ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn), }
)) _ => ImplTraitContext::disallowed(),
} };
_ => hir::FunctionRetTy::Return( hir::FunctionRetTy::Return(self.lower_ty(ty, context))
self.lower_ty(ty, ImplTraitContext::disallowed()), }
),
},
FunctionRetTy::Default(span) => hir::FunctionRetTy::DefaultReturn(span), FunctionRetTy::Default(span) => hir::FunctionRetTy::DefaultReturn(span),
} }
}; };
@ -1961,10 +1959,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) -> hir::GenericBound<'hir> { ) -> hir::GenericBound<'hir> {
// Compute the `T` in `Future<Output = T>` from the return type. // Compute the `T` in `Future<Output = T>` from the return type.
let output_ty = match output { let output_ty = match output {
FunctionRetTy::Ty(ty) => self.lower_ty( FunctionRetTy::Ty(ty) => {
ty, // Not `OpaqueTyOrigin::AsyncFn`: that's only used for the
ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn), // `impl Future` opaque type that `async fn` implicitly
), // generates.
let context =
ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn);
self.lower_ty(ty, context)
}
FunctionRetTy::Default(ret_ty_span) => self.arena.alloc(self.ty_tup(*ret_ty_span, &[])), FunctionRetTy::Default(ret_ty_span) => self.arena.alloc(self.ty_tup(*ret_ty_span, &[])),
}; };

View file

@ -120,7 +120,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.mir_def_id, self.mir_def_id,
Locations::All(output_span), Locations::All(output_span),
ConstraintCategory::BoringNoLocation, ConstraintCategory::BoringNoLocation,
true,
) { ) {
span_mirbug!( span_mirbug!(
self, self,
@ -144,7 +143,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.mir_def_id, self.mir_def_id,
Locations::All(output_span), Locations::All(output_span),
ConstraintCategory::BoringNoLocation, ConstraintCategory::BoringNoLocation,
false,
) { ) {
span_mirbug!( span_mirbug!(
self, self,

View file

@ -1122,14 +1122,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// the resulting inferend values are stored with the // the resulting inferend values are stored with the
// def-id of the base function. // def-id of the base function.
let parent_def_id = self.tcx().closure_base_def_id(self.mir_def_id); let parent_def_id = self.tcx().closure_base_def_id(self.mir_def_id);
return self.eq_opaque_type_and_type( return self.eq_opaque_type_and_type(sub, sup, parent_def_id, locations, category);
sub,
sup,
parent_def_id,
locations,
category,
false,
);
} else { } else {
return Err(terr); return Err(terr);
} }
@ -1195,7 +1188,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
anon_owner_def_id: DefId, anon_owner_def_id: DefId,
locations: Locations, locations: Locations,
category: ConstraintCategory, category: ConstraintCategory,
is_function_return: bool,
) -> Fallible<()> { ) -> Fallible<()> {
debug!( debug!(
"eq_opaque_type_and_type( \ "eq_opaque_type_and_type( \
@ -1273,13 +1265,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
opaque_decl.concrete_ty, resolved_ty, renumbered_opaque_defn_ty, opaque_decl.concrete_ty, resolved_ty, renumbered_opaque_defn_ty,
); );
if !concrete_is_opaque if !concrete_is_opaque {
|| (is_function_return
&& matches!(opaque_decl.origin, hir::OpaqueTyOrigin::FnReturn))
{
// For return position impl Trait, the function
// return is the only possible definition site, so
// always record it.
obligations.add( obligations.add(
infcx infcx
.at(&ObligationCause::dummy(), param_env) .at(&ObligationCause::dummy(), param_env)

View file

@ -1471,7 +1471,9 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
&tcx.mir_borrowck(owner).concrete_opaque_types &tcx.mir_borrowck(owner).concrete_opaque_types
} }
hir::OpaqueTyOrigin::Misc => { hir::OpaqueTyOrigin::Misc => {
// We shouldn't leak borrowck results through impl Trait in bindings. // We shouldn't leak borrowck results through impl trait in bindings.
// For example, we shouldn't be able to tell if `x` in
// `let x: impl Sized + 'a = &()` has type `&'static ()` or `&'a ()`.
&tcx.typeck_tables_of(owner).concrete_opaque_types &tcx.typeck_tables_of(owner).concrete_opaque_types
} }
hir::OpaqueTyOrigin::TypeAlias => { hir::OpaqueTyOrigin::TypeAlias => {
@ -1482,9 +1484,6 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
.get(&def_id) .get(&def_id)
.map(|opaque| opaque.concrete_type) .map(|opaque| opaque.concrete_type)
.unwrap_or_else(|| { .unwrap_or_else(|| {
// This can occur if some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.sess.delay_span_bug( tcx.sess.delay_span_bug(
DUMMY_SP, DUMMY_SP,
&format!( &format!(
@ -1492,7 +1491,20 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
owner, def_id, owner, def_id,
), ),
); );
tcx.types.err if tcx.typeck_tables_of(owner).tainted_by_errors {
// Some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.types.err
} else {
// We failed to resolve the opaque type or it
// resolves to itself. Return the non-revealed
// type, which should result in E0720.
tcx.mk_opaque(
def_id,
InternalSubsts::identity_for_item(tcx, def_id),
)
}
}); });
debug!("concrete_ty = {:?}", concrete_ty); debug!("concrete_ty = {:?}", concrete_ty);
if concrete_ty.has_erased_regions() { if concrete_ty.has_erased_regions() {

View file

@ -131,6 +131,9 @@ fn enforce_impl_params_are_constrained(
} }
} }
ty::AssocKind::OpaqueTy => { ty::AssocKind::OpaqueTy => {
// We don't know which lifetimes appear in the actual
// opaque type, so use all of the lifetimes that appear
// in the type's predicates.
let predicates = tcx.predicates_of(def_id).instantiate_identity(tcx); let predicates = tcx.predicates_of(def_id).instantiate_identity(tcx);
cgp::parameters_for(&predicates, true) cgp::parameters_for(&predicates, true)
} }