Skip to content

Commit 8e15cf7

Browse files
matthewjaspermatthewjasper
committed
Address review comments
1 parent dff459d commit 8e15cf7

File tree

6 files changed

+61
-53
lines changed

6 files changed

+61
-53
lines changed

‎src/librustc/infer/opaque_types/mod.rs

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -502,9 +502,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
502502
hir::OpaqueTyOrigin::AsyncFn => return false,
503503

504504
// Otherwise, generate the label we'll use in the error message.
505-
hir::OpaqueTyOrigin::TypeAlias => "impl Trait",
506-
hir::OpaqueTyOrigin::FnReturn => "impl Trait",
507-
hir::OpaqueTyOrigin::Misc => "impl Trait",
505+
hir::OpaqueTyOrigin::TypeAlias
506+
| hir::OpaqueTyOrigin::FnReturn
507+
| hir::OpaqueTyOrigin::Misc => "impl Trait",
508508
};
509509
let msg = format!("ambiguous lifetime bound in `{}`", context_name);
510510
let mut err = self.tcx.sess.struct_span_err(span, &msg);
@@ -815,18 +815,25 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> {
815815

816816
fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
817817
match r {
818-
// Ignore bound regions that appear in the type, they don't need to
819-
// be remapped (e.g., this would ignore `'r` in a type like
820-
// `for<'r> fn(&'r u32)`.
821-
ty::ReLateBound(..)
822-
823-
// If regions have been erased, don't try to unerase them.
824-
| ty::ReErased
825-
826-
// ignore `'static`, as that can appear anywhere
827-
| ty::ReStatic => return r,
828-
829-
_ => {}
818+
// Ignore bound regions and `'static` regions that appear in the
819+
// type, we only need to remap regions that reference lifetimes
820+
// from the function declaraion.
821+
// This would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
822+
ty::ReLateBound(..) | ty::ReStatic => return r,
823+
824+
// If regions have been erased (by writeback), don't try to unerase
825+
// them.
826+
ty::ReErased => return r,
827+
828+
// The regions that we expect from borrow checking.
829+
ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReEmpty => {}
830+
831+
ty::RePlaceholder(_) | ty::ReVar(_) | ty::ReScope(_) | ty::ReClosureBound(_) => {
832+
// All of the regions in the type should either have been
833+
// erased by writeback, or mapped back to named regions by
834+
// borrow checking.
835+
bug!("unexpected region kind in opaque type: {:?}", r);
836+
}
830837
}
831838

832839
let generics = self.tcx().generics_of(self.opaque_type_def_id);

‎src/librustc_ast_lowering/lib.rs

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,17 +1728,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
17281728
)
17291729
} else {
17301730
match decl.output {
1731-
FunctionRetTy::Ty(ref ty) => match in_band_ty_params {
1732-
Some((def_id, _)) if impl_trait_return_allow => {
1733-
hir::FunctionRetTy::Return(self.lower_ty(
1734-
ty,
1735-
ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn),
1736-
))
1737-
}
1738-
_ => hir::FunctionRetTy::Return(
1739-
self.lower_ty(ty, ImplTraitContext::disallowed()),
1740-
),
1741-
},
1731+
FunctionRetTy::Ty(ref ty) => {
1732+
let context = match in_band_ty_params {
1733+
Some((def_id, _)) if impl_trait_return_allow => {
1734+
ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn)
1735+
}
1736+
_ => ImplTraitContext::disallowed(),
1737+
};
1738+
hir::FunctionRetTy::Return(self.lower_ty(ty, context))
1739+
}
17421740
FunctionRetTy::Default(span) => hir::FunctionRetTy::DefaultReturn(span),
17431741
}
17441742
};
@@ -1966,10 +1964,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
19661964
) -> hir::GenericBound<'hir> {
19671965
// Compute the `T` in `Future<Output = T>` from the return type.
19681966
let output_ty = match output {
1969-
FunctionRetTy::Ty(ty) => self.lower_ty(
1970-
ty,
1971-
ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn),
1972-
),
1967+
FunctionRetTy::Ty(ty) => {
1968+
// Not `OpaqueTyOrigin::AsyncFn`: that's only used for the
1969+
// `impl Future` opaque type that `async fn` implicitly
1970+
// generates.
1971+
let context =
1972+
ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn);
1973+
self.lower_ty(ty, context)
1974+
}
19731975
FunctionRetTy::Default(ret_ty_span) => self.arena.alloc(self.ty_tup(*ret_ty_span, &[])),
19741976
};
19751977

‎src/librustc_mir/borrow_check/type_check/input_output.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
120120
self.mir_def_id,
121121
Locations::All(output_span),
122122
ConstraintCategory::BoringNoLocation,
123-
true,
124123
) {
125124
span_mirbug!(
126125
self,
@@ -144,7 +143,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
144143
self.mir_def_id,
145144
Locations::All(output_span),
146145
ConstraintCategory::BoringNoLocation,
147-
false,
148146
) {
149147
span_mirbug!(
150148
self,

‎src/librustc_mir/borrow_check/type_check/mod.rs

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,14 +1123,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
11231123
// the resulting inferend values are stored with the
11241124
// def-id of the base function.
11251125
let parent_def_id = self.tcx().closure_base_def_id(self.mir_def_id);
1126-
return self.eq_opaque_type_and_type(
1127-
sub,
1128-
sup,
1129-
parent_def_id,
1130-
locations,
1131-
category,
1132-
false,
1133-
);
1126+
return self.eq_opaque_type_and_type(sub, sup, parent_def_id, locations, category);
11341127
} else {
11351128
return Err(terr);
11361129
}
@@ -1196,7 +1189,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
11961189
anon_owner_def_id: DefId,
11971190
locations: Locations,
11981191
category: ConstraintCategory,
1199-
is_function_return: bool,
12001192
) -> Fallible<()> {
12011193
debug!(
12021194
"eq_opaque_type_and_type( \
@@ -1274,13 +1266,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
12741266
opaque_decl.concrete_ty, resolved_ty, renumbered_opaque_defn_ty,
12751267
);
12761268

1277-
if !concrete_is_opaque
1278-
|| (is_function_return
1279-
&& matches!(opaque_decl.origin, hir::OpaqueTyOrigin::FnReturn))
1280-
{
1281-
// For return position impl Trait, the function
1282-
// return is the only possible definition site, so
1283-
// always record it.
1269+
if !concrete_is_opaque {
12841270
obligations.add(
12851271
infcx
12861272
.at(&ObligationCause::dummy(), param_env)

‎src/librustc_typeck/collect.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,9 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
13601360
&tcx.mir_borrowck(owner).concrete_opaque_types
13611361
}
13621362
hir::OpaqueTyOrigin::Misc => {
1363-
// We shouldn't leak borrowck results through impl Trait in bindings.
1363+
// We shouldn't leak borrowck results through impl trait in bindings.
1364+
// For example, we shouldn't be able to tell if `x` in
1365+
// `let x: impl Sized + 'a = &()` has type `&'static ()` or `&'a ()`.
13641366
&tcx.typeck_tables_of(owner).concrete_opaque_types
13651367
}
13661368
hir::OpaqueTyOrigin::TypeAlias => {
@@ -1371,17 +1373,27 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
13711373
.get(&def_id)
13721374
.map(|opaque| opaque.concrete_type)
13731375
.unwrap_or_else(|| {
1374-
// This can occur if some error in the
1375-
// owner fn prevented us from populating
1376-
// the `concrete_opaque_types` table.
13771376
tcx.sess.delay_span_bug(
13781377
DUMMY_SP,
13791378
&format!(
13801379
"owner {:?} has no opaque type for {:?} in its tables",
13811380
owner, def_id,
13821381
),
13831382
);
1384-
tcx.types.err
1383+
if tcx.typeck_tables_of(owner).tainted_by_errors {
1384+
// Some error in the
1385+
// owner fn prevented us from populating
1386+
// the `concrete_opaque_types` table.
1387+
tcx.types.err
1388+
} else {
1389+
// We failed to resolve the opaque type or it
1390+
// resolves to itself. Return the non-revealed
1391+
// type, which should result in E0720.
1392+
tcx.mk_opaque(
1393+
def_id,
1394+
InternalSubsts::identity_for_item(tcx, def_id),
1395+
)
1396+
}
13851397
});
13861398
debug!("concrete_ty = {:?}", concrete_ty);
13871399
if concrete_ty.has_erased_regions() {

‎src/librustc_typeck/impl_wf_check.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ fn enforce_impl_params_are_constrained(
133133
}
134134
}
135135
ty::AssocKind::OpaqueTy => {
136+
// We don't know which lifetimes appear in the actual
137+
// opaque type, so use all of the lifetimes that appear
138+
// in the type's predicates.
136139
let predicates = tcx.predicates_of(def_id).instantiate_identity(tcx);
137140
cgp::parameters_for(&predicates, true)
138141
}

0 commit comments

Comments
 (0)