Auto merge of #63376 - nikomatsakis:async-await-issue-62517, r=<try>

use different lifetime name for object-lifetime-default elision

Introduce a distinct value for `LifetimeName` to use when this is a object-lifetime-default elision. This allows us to avoid creating incorrect lifetime parameters for the opaque types that result. We really need to overhaul this setup at some point! It's getting increasingly byzantine. But this seems like a relatively... surgical fix.

r? @cramertj

Fixes #62517

Author: bors

src/librustc/hir/intravisit.rs

@@ -433,6 +433,7 @@ pub fn walk_lifetime<'v, V: Visitor<'v>>(visitor: &mut V, lifetime: &'v Lifetime
         LifetimeName::Static |
         LifetimeName::Error |
         LifetimeName::Implicit |
+        LifetimeName::ImplicitObjectLifetimeDefault |
         LifetimeName::Underscore => {}
     }
 }

src/librustc/hir/lowering.rs

@@ -319,7 +319,7 @@ enum ParenthesizedGenericArgs {
 /// `resolve_lifetime` module. Often we "fallthrough" to that code by generating
 /// an "elided" or "underscore" lifetime name. In the future, we probably want to move
 /// everything into HIR lowering.
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 enum AnonymousLifetimeMode {
     /// For **Modern** cases, create a new anonymous region parameter
     /// and reference that.
@@ -357,7 +357,7 @@ enum AnonymousLifetimeMode {
 }
 
 /// The type of elided lifetime replacement to perform on `async fn` return types.
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 enum LtReplacement {
     /// Fresh name introduced by the single non-dyn elided lifetime
     /// in the arguments of the async fn.
@@ -756,10 +756,16 @@ impl<'a> LoweringContext<'a> {
         anonymous_lifetime_mode: AnonymousLifetimeMode,
         op: impl FnOnce(&mut Self) -> R,
     ) -> R {
+        debug!(
+            "with_anonymous_lifetime_mode(anonymous_lifetime_mode={:?})",
+            anonymous_lifetime_mode,
+        );
         let old_anonymous_lifetime_mode = self.anonymous_lifetime_mode;
         self.anonymous_lifetime_mode = anonymous_lifetime_mode;
         let result = op(self);
         self.anonymous_lifetime_mode = old_anonymous_lifetime_mode;
+        debug!("with_anonymous_lifetime_mode: restoring anonymous_lifetime_mode={:?}",
+               old_anonymous_lifetime_mode);
         result
     }
 
@@ -1396,6 +1402,13 @@ impl<'a> LoweringContext<'a> {
         opaque_ty_node_id: NodeId,
         lower_bounds: impl FnOnce(&mut LoweringContext<'_>) -> hir::GenericBounds,
     ) -> hir::TyKind {
+        debug!(
+            "lower_opaque_impl_trait(fn_def_id={:?}, opaque_ty_node_id={:?}, span={:?})",
+            fn_def_id,
+            opaque_ty_node_id,
+            span,
+        );
+
         // Make sure we know that some funky desugaring has been going on here.
         // This is a first: there is code in other places like for loop
         // desugaring that explicitly states that we don't want to track that.
@@ -1423,6 +1436,14 @@ impl<'a> LoweringContext<'a> {
             &hir_bounds,
         );
 
+        debug!(
+            "lower_opaque_impl_trait: lifetimes={:#?}", lifetimes,
+        );
+
+        debug!(
+            "lower_opaque_impl_trait: lifetime_defs={:#?}", lifetime_defs,
+        );
+
         self.with_hir_id_owner(opaque_ty_node_id, |lctx| {
             let opaque_ty_item = hir::OpaqueTy {
                 generics: hir::Generics {
@@ -1438,7 +1459,7 @@ impl<'a> LoweringContext<'a> {
                 origin: hir::OpaqueTyOrigin::FnReturn,
             };
 
-            trace!("exist ty from impl trait def-index: {:#?}", opaque_ty_def_index);
+            trace!("lower_opaque_impl_trait: {:#?}", opaque_ty_def_index);
             let opaque_ty_id = lctx.generate_opaque_type(
                 opaque_ty_node_id,
                 opaque_ty_item,
@@ -1486,6 +1507,13 @@ impl<'a> LoweringContext<'a> {
         parent_index: DefIndex,
         bounds: &hir::GenericBounds,
     ) -> (HirVec<hir::GenericArg>, HirVec<hir::GenericParam>) {
+        debug!(
+            "lifetimes_from_impl_trait_bounds(opaque_ty_id={:?}, \
+             parent_index={:?}, \
+             bounds={:#?})",
+            opaque_ty_id, parent_index, bounds,
+        );
+
         // This visitor walks over `impl Trait` bounds and creates defs for all lifetimes that
         // appear in the bounds, excluding lifetimes that are created within the bounds.
         // E.g., `'a`, `'b`, but not `'c` in `impl for<'c> SomeTrait<'a, 'b, 'c>`.
@@ -1573,6 +1601,11 @@ impl<'a> LoweringContext<'a> {
                         }
                     }
                     hir::LifetimeName::Param(_) => lifetime.name,
+
+                    // Refers to some other lifetime that is "in
+                    // scope" within the type.
+                    hir::LifetimeName::ImplicitObjectLifetimeDefault => return,
+
                     hir::LifetimeName::Error | hir::LifetimeName::Static => return,
                 };
 
@@ -2232,6 +2265,15 @@ impl<'a> LoweringContext<'a> {
         opaque_ty_node_id: NodeId,
         elided_lt_replacement: LtReplacement,
     ) -> hir::FunctionRetTy {
+        debug!(
+            "lower_async_fn_ret_ty(\
+             output={:?}, \
+             fn_def_id={:?}, \
+             opaque_ty_node_id={:?}, \
+             elided_lt_replacement={:?})",
+            output, fn_def_id, opaque_ty_node_id, elided_lt_replacement,
+        );
+
         let span = output.span();
 
         let opaque_ty_span = self.mark_span_with_reason(
@@ -2258,6 +2300,8 @@ impl<'a> LoweringContext<'a> {
                 ),
             );
 
+            debug!("lower_async_fn_ret_ty: future_bound={:#?}", future_bound);
+
             // Calculate all the lifetimes that should be captured
             // by the opaque type. This should include all in-scope
             // lifetime parameters, including those defined in-band.
@@ -2507,6 +2551,12 @@ impl<'a> LoweringContext<'a> {
                     hir::LifetimeName::Implicit
                         | hir::LifetimeName::Underscore
                         | hir::LifetimeName::Static => hir::ParamName::Plain(lt.name.ident()),
+                    hir::LifetimeName::ImplicitObjectLifetimeDefault => {
+                        span_bug!(
+                            param.ident.span,
+                            "object-lifetime-default should not occur here",
+                        );
+                    }
                     hir::LifetimeName::Error => ParamName::Error,
                 };
 
@@ -3273,7 +3323,13 @@ impl<'a> LoweringContext<'a> {
             AnonymousLifetimeMode::Replace(_) => {}
         }
 
-        self.new_implicit_lifetime(span)
+        let r = hir::Lifetime {
+            hir_id: self.next_id(),
+            span,
+            name: hir::LifetimeName::ImplicitObjectLifetimeDefault,
+        };
+        debug!("elided_dyn_bound: r={:?}", r);
+        r
     }
 
     fn new_replacement_lifetime(

src/librustc/hir/mod.rs

@@ -222,6 +222,19 @@ pub enum LifetimeName {
     /// User wrote nothing (e.g., the lifetime in `&u32`).
     Implicit,
 
+    /// Implicit lifetime in a context like `dyn Foo`. This is
+    /// distinguished from implicit lifetimes elsewhere because the
+    /// lifetime that they default to must appear elsewhere within the
+    /// enclosing type.  This means that, in an `impl Trait` context, we
+    /// don't have to create a parameter for them. That is, `impl
+    /// Trait<Item = &u32>` expands to an opaque type like `type
+    /// Foo<'a> = impl Trait<Item = &'a u32>`, but `impl Trait<item =
+    /// dyn Bar>` expands to `type Foo = impl Trait<Item = dyn Bar +
+    /// 'static>`. The latter uses `ImplicitObjectLifetimeDefault` so
+    /// that surrounding code knows not to create a lifetime
+    /// parameter.
+    ImplicitObjectLifetimeDefault,
+
     /// Indicates an error during lowering (usually `'_` in wrong place)
     /// that was already reported.
     Error,
@@ -236,7 +249,9 @@ pub enum LifetimeName {
 impl LifetimeName {
     pub fn ident(&self) -> Ident {
         match *self {
-            LifetimeName::Implicit | LifetimeName::Error => Ident::invalid(),
+            LifetimeName::ImplicitObjectLifetimeDefault
+            | LifetimeName::Implicit
+            | LifetimeName::Error => Ident::invalid(),
             LifetimeName::Underscore => Ident::with_empty_ctxt(kw::UnderscoreLifetime),
             LifetimeName::Static => Ident::with_empty_ctxt(kw::StaticLifetime),
             LifetimeName::Param(param_name) => param_name.ident(),
@@ -245,7 +260,9 @@ impl LifetimeName {
 
     pub fn is_elided(&self) -> bool {
         match self {
-            LifetimeName::Implicit | LifetimeName::Underscore => true,
+            LifetimeName::ImplicitObjectLifetimeDefault
+            | LifetimeName::Implicit
+            | LifetimeName::Underscore => true,
 
             // It might seem surprising that `Fresh(_)` counts as
             // *not* elided -- but this is because, as far as the code

src/librustc/infer/opaque_types/mod.rs

@@ -1108,6 +1108,7 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         // Use the same type variable if the exact same opaque type appears more
         // than once in the return type (e.g., if it's passed to a type alias).
         if let Some(opaque_defn) = self.opaque_types.get(&def_id) {
+            debug!("instantiate_opaque_types: returning concrete ty {:?}", opaque_defn.concrete_ty);
             return opaque_defn.concrete_ty;
         }
         let span = tcx.def_span(def_id);

src/librustc/middle/resolve_lifetime.rs

@@ -5,6 +5,8 @@
 //! used between functions, and they operate in a purely top-down
 //! way. Therefore, we break lifetime name resolution into a separate pass.
 
+// ignore-tidy-filelength
+
 use crate::hir::def::{Res, DefKind};
 use crate::hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
 use crate::hir::map::Map;
@@ -556,6 +558,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
 
     fn visit_ty(&mut self, ty: &'tcx hir::Ty) {
         debug!("visit_ty: id={:?} ty={:?}", ty.hir_id, ty);
+        debug!("visit_ty: ty.node={:?}", ty.node);
         match ty.node {
             hir::TyKind::BareFn(ref c) => {
                 let next_early_index = self.next_early_index();
@@ -585,11 +588,20 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                 self.is_in_fn_syntax = was_in_fn_syntax;
             }
             hir::TyKind::TraitObject(ref bounds, ref lifetime) => {
+                debug!("visit_ty: TraitObject(bounds={:?}, lifetime={:?})", bounds, lifetime);
                 for bound in bounds {
                     self.visit_poly_trait_ref(bound, hir::TraitBoundModifier::None);
                 }
                 match lifetime.name {
                     LifetimeName::Implicit => {
+                        // For types like `dyn Foo`, we should
+                        // generate a special form of elided.
+                        span_bug!(
+                            ty.span,
+                            "object-lifetime-default expected, not implict",
+                        );
+                    }
+                    LifetimeName::ImplicitObjectLifetimeDefault => {
                         // If the user does not write *anything*, we
                         // use the object lifetime defaulting
                         // rules. So e.g., `Box<dyn Debug>` becomes
@@ -897,6 +909,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
     }
 
     fn visit_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
+        debug!("visit_lifetime(lifetime_ref={:?})", lifetime_ref);
         if lifetime_ref.is_elided() {
             self.resolve_elided_lifetimes(vec![lifetime_ref]);
             return;
@@ -1911,6 +1924,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
     }
 
     fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir::GenericArgs) {
+        debug!(
+            "visit_segment_args(res={:?}, depth={:?}, generic_args={:?})",
+            res,
+            depth,
+            generic_args,
+        );
+
         if generic_args.parenthesized {
             let was_in_fn_syntax = self.is_in_fn_syntax;
             self.is_in_fn_syntax = true;
@@ -1964,6 +1984,23 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
             _ => None,
         };
 
+        debug!("visit_segment_args: type_def_id={:?}", type_def_id);
+
+        // Compute a vector of defaults, one for each type parameter,
+        // per the rules given in RFCs 599 and 1156. Example:
+        //
+        // ```rust
+        // struct Foo<'a, T: 'a, U> { }
+        // ```
+        //
+        // If you have `Foo<'x, dyn Bar, dyn Baz>`, we want to default
+        // `dyn Bar` to `dyn Bar + 'x` (because of the `T: 'a` bound)
+        // and `dyn Baz` to `dyn Baz + 'static` (because there is no
+        // such bound).
+        //
+        // Therefore, we would compute `object_lifetime_defaults` to a
+        // vector like `['x, 'static]`. Note that the vector only
+        // includes type parameters.
         let object_lifetime_defaults = type_def_id.map_or(vec![], |def_id| {
             let in_body = {
                 let mut scope = self.scope;
@@ -2003,6 +2040,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                             .collect()
                     })
             };
+            debug!("visit_segment_args: unsubst={:?}", unsubst);
             unsubst
                 .iter()
                 .map(|set| match *set {
@@ -2023,6 +2061,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                 .collect()
         });
 
+        debug!("visit_segment_args: object_lifetime_defaults={:?}", object_lifetime_defaults);
+
         let mut i = 0;
         for arg in &generic_args.args {
             match arg {
@@ -2045,8 +2085,49 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
             }
         }
 
+        // Hack: when resolving the type `XX` in binding like `dyn
+        // Foo<'b, Item = XX>`, the current object-lifetime default
+        // would be to examine the trait `Foo` to check whether it has
+        // a lifetime bound declared on `Item`. e.g., if `Foo` is
+        // declared like so, then the default object lifetime bound in
+        // `XX` should be `'b`:
+        //
+        // ```rust
+        // trait Foo<'a> {
+        //   type Item: 'a;
+        // }
+        // ```
+        //
+        // but if we just have `type Item;`, then it would be
+        // `'static`. However, we don't get all of this logic correct.
+        //
+        // Instead, we do something hacky: if there are no lifetime parameters
+        // to the trait, then we simply use a default object lifetime
+        // bound of `'static`, because there is no other possibility. On the other hand,
+        // if there ARE lifetime parameters, then we require the user to give an
+        // explicit bound for now.
+        //
+        // This is intended to leave room for us to implement the
+        // correct behavior in the future.
+        let has_lifetime_parameter = generic_args
+            .args
+            .iter()
+            .any(|arg| match arg {
+                GenericArg::Lifetime(_) => true,
+                _ => false,
+            });
+
+        // Resolve lifetimes found in the type `XX` from `Item = XX` bindings.
         for b in &generic_args.bindings {
-            self.visit_assoc_type_binding(b);
+            let scope = Scope::ObjectLifetimeDefault {
+                lifetime: if has_lifetime_parameter {
+                    None
+                } else {
+                    Some(Region::Static)
+                },
+                s: self.scope,
+            };
+            self.with(scope, |_, this| this.visit_assoc_type_binding(b));
         }
     }
 
@@ -2347,6 +2428,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
     }
 
     fn resolve_elided_lifetimes(&mut self, lifetime_refs: Vec<&'tcx hir::Lifetime>) {
+        debug!("resolve_elided_lifetimes(lifetime_refs={:?})", lifetime_refs);
+
         if lifetime_refs.is_empty() {
             return;
         }
@@ -2539,6 +2622,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
     }
 
     fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
+        debug!("resolve_object_lifetime_default(lifetime_ref={:?})", lifetime_ref);
         let mut late_depth = 0;
         let mut scope = self.scope;
         let lifetime = loop {
@@ -2638,6 +2722,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                         hir::LifetimeName::Param(_) | hir::LifetimeName::Implicit => {
                             self.resolve_lifetime_ref(lt);
                         }
+                        hir::LifetimeName::ImplicitObjectLifetimeDefault => {
+                            self.tcx.sess.delay_span_bug(
+                                lt.span,
+                                "lowering generated `ImplicitObjectLifetimeDefault` \
+                                 outside of an object type",
+                            )
+                        }
                         hir::LifetimeName::Error => {
                             // No need to do anything, error already reported.
                         }