Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Feb 14, 2020
1 parent 93ac5bc commit f23bca7
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 55 deletions.
45 changes: 28 additions & 17 deletions src/librustc/infer/opaque_types/mod.rs
Expand Up @@ -500,9 +500,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
hir::OpaqueTyOrigin::AsyncFn => return false,

// Otherwise, generate the label we'll use in the error message.
hir::OpaqueTyOrigin::TypeAlias => "impl Trait",
hir::OpaqueTyOrigin::FnReturn => "impl Trait",
hir::OpaqueTyOrigin::Misc => "impl Trait",
hir::OpaqueTyOrigin::TypeAlias
| hir::OpaqueTyOrigin::FnReturn
| hir::OpaqueTyOrigin::Misc => "impl Trait",
};
let msg = format!("ambiguous lifetime bound in `{}`", context_name);
let mut err = self.tcx.sess.struct_span_err(span, &msg);
Expand Down Expand Up @@ -814,26 +814,37 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {

fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
match r {
// Ignore bound regions that appear in the type, they don't need to
// be remapped (e.g., this would ignore `'r` in a type like
// `for<'r> fn(&'r u32)`.
ty::ReLateBound(..)

// If regions have been erased, don't try to unerase them.
| ty::ReErased

// ignore `'static`, as that can appear anywhere
| ty::ReStatic => return r,

_ => {}
// Ignore bound regions and `'static` regions that appear in the
// type, we only need to remap regions that reference lifetimes
// from the function declaraion.
// This would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
ty::ReLateBound(..) | ty::ReStatic => return r,

// If regions have been erased (by writeback), don't try to unerase
// them.
ty::ReErased => return r,

// The regions that we expect from borrow checking.
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);
match self.map.get(&r.into()).map(|k| k.unpack()) {
Some(GenericArgKind::Lifetime(r1)) => r1,
Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
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() => {
if let Some(hidden_ty) = self.hidden_ty.take() {
Expand Down Expand Up @@ -862,7 +873,7 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
)
.emit();

self.tcx().mk_region(ty::ReStatic)
self.tcx().lifetimes.re_static
}
}
}
Expand Down
32 changes: 17 additions & 15 deletions src/librustc_ast_lowering/lib.rs
Expand Up @@ -1723,17 +1723,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
)
} else {
match decl.output {
FunctionRetTy::Ty(ref ty) => match in_band_ty_params {
Some((def_id, _)) if impl_trait_return_allow => {
hir::FunctionRetTy::Return(self.lower_ty(
ty,
ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn),
))
}
_ => hir::FunctionRetTy::Return(
self.lower_ty(ty, ImplTraitContext::disallowed()),
),
},
FunctionRetTy::Ty(ref ty) => {
let context = match in_band_ty_params {
Some((def_id, _)) if impl_trait_return_allow => {
ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn)
}
_ => ImplTraitContext::disallowed(),
};
hir::FunctionRetTy::Return(self.lower_ty(ty, context))
}
FunctionRetTy::Default(span) => hir::FunctionRetTy::DefaultReturn(span),
}
};
Expand Down Expand Up @@ -1961,10 +1959,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) -> hir::GenericBound<'hir> {
// Compute the `T` in `Future<Output = T>` from the return type.
let output_ty = match output {
FunctionRetTy::Ty(ty) => self.lower_ty(
ty,
ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn),
),
FunctionRetTy::Ty(ty) => {
// Not `OpaqueTyOrigin::AsyncFn`: that's only used for the
// `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, &[])),
};

Expand Down
2 changes: 0 additions & 2 deletions src/librustc_mir/borrow_check/type_check/input_output.rs
Expand Up @@ -120,7 +120,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.mir_def_id,
Locations::All(output_span),
ConstraintCategory::BoringNoLocation,
true,
) {
span_mirbug!(
self,
Expand All @@ -144,7 +143,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.mir_def_id,
Locations::All(output_span),
ConstraintCategory::BoringNoLocation,
false,
) {
span_mirbug!(
self,
Expand Down
18 changes: 2 additions & 16 deletions src/librustc_mir/borrow_check/type_check/mod.rs
Expand Up @@ -1122,14 +1122,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// the resulting inferend values are stored with the
// def-id of the base function.
let parent_def_id = self.tcx().closure_base_def_id(self.mir_def_id);
return self.eq_opaque_type_and_type(
sub,
sup,
parent_def_id,
locations,
category,
false,
);
return self.eq_opaque_type_and_type(sub, sup, parent_def_id, locations, category);
} else {
return Err(terr);
}
Expand Down Expand Up @@ -1195,7 +1188,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
anon_owner_def_id: DefId,
locations: Locations,
category: ConstraintCategory,
is_function_return: bool,
) -> Fallible<()> {
debug!(
"eq_opaque_type_and_type( \
Expand Down Expand Up @@ -1273,13 +1265,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
opaque_decl.concrete_ty, resolved_ty, renumbered_opaque_defn_ty,
);

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.
if !concrete_is_opaque {
obligations.add(
infcx
.at(&ObligationCause::dummy(), param_env)
Expand Down
22 changes: 17 additions & 5 deletions src/librustc_typeck/collect.rs
Expand Up @@ -1471,7 +1471,9 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
&tcx.mir_borrowck(owner).concrete_opaque_types
}
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
}
hir::OpaqueTyOrigin::TypeAlias => {
Expand All @@ -1482,17 +1484,27 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
.get(&def_id)
.map(|opaque| opaque.concrete_type)
.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(
DUMMY_SP,
&format!(
"owner {:?} has no opaque type for {:?} in its tables",
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);
if concrete_ty.has_erased_regions() {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_typeck/impl_wf_check.rs
Expand Up @@ -131,6 +131,9 @@ fn enforce_impl_params_are_constrained(
}
}
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);
cgp::parameters_for(&predicates, true)
}
Expand Down

0 comments on commit f23bca7

Please sign in to comment.