Address review comments

Author: matthewjasper

src/librustc/infer/opaque_types/mod.rs

@@ -502,9 +502,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);
@@ -815,18 +815,25 @@ 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::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);

src/librustc_ast_lowering/lib.rs

@@ -1728,17 +1728,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),
             }
         };
@@ -1966,10 +1964,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, &[])),
         };
 

src/librustc_mir/borrow_check/type_check/input_output.rs

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

src/librustc_mir/borrow_check/type_check/mod.rs

@@ -1123,14 +1123,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);
             }
@@ -1196,7 +1189,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( \
@@ -1274,13 +1266,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)

src/librustc_typeck/collect.rs

@@ -1360,7 +1360,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 => {
@@ -1371,17 +1373,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() {

src/librustc_typeck/impl_wf_check.rs

@@ -133,6 +133,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)
                 }