Reject unconstrained lifetimes in type_of(assoc_ty) instead of during…

… wfcheck of the impl item

compiler/rustc_hir_analysis/messages.ftl

@@ -540,6 +540,7 @@ hir_analysis_unconstrained_generic_parameter = the {$param_def_kind} `{$param_na
     .label = unconstrained {$param_def_kind}
     .const_param_note = expressions using a const parameter must map each value to a distinct output value
     .const_param_note2 = proving the result of expressions other than the parameter are unique is not supported
+    .help = this use of an otherwise unconstrained lifetime is not allowed
 
 hir_analysis_unconstrained_opaque_type = unconstrained opaque type
     .note = `{$name}` must be used in combination with a concrete type within the same {$what}

compiler/rustc_hir_analysis/src/collect.rs

@@ -16,10 +16,11 @@
 
 use rustc_ast::Recovered;
 use rustc_data_structures::captures::Captures;
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_data_structures::unord::UnordMap;
 use rustc_errors::{
-    struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0228,
+    struct_span_code_err, Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, StashKey, E0207,
+    E0228,
 };
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
@@ -31,7 +32,9 @@ use rustc_infer::traits::ObligationCause;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::util::{Discr, IntTypeExt};
-use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, Upcast};
+use rustc_middle::ty::{
+    self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt as _, Upcast,
+};
 use rustc_middle::{bug, span_bug};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{Span, DUMMY_SP};
@@ -40,11 +43,13 @@ use rustc_trait_selection::error_reporting::traits::suggestions::NextTypeParamNa
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::ObligationCtxt;
 use std::cell::Cell;
+use std::cell::RefCell;
 use std::iter;
 use std::ops::Bound;
 
 use crate::check::intrinsic::intrinsic_operation_unsafety;
-use crate::errors;
+use crate::constrained_generic_params::{self as cgp, Parameter};
+use crate::errors::{self, UnconstrainedGenericParameter};
 use crate::hir_ty_lowering::{HirTyLowerer, RegionInferReason};
 
 pub(crate) mod dump;
@@ -121,6 +126,7 @@ pub struct ItemCtxt<'tcx> {
     tcx: TyCtxt<'tcx>,
     item_def_id: LocalDefId,
     tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
+    pub forbidden_params: RefCell<FxHashMap<Parameter, ty::GenericParamDef>>,
 }
 
 ///////////////////////////////////////////////////////////////////////////
@@ -353,7 +359,12 @@ fn bad_placeholder<'cx, 'tcx>(
 
 impl<'tcx> ItemCtxt<'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
-        ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
+        ItemCtxt {
+            tcx,
+            item_def_id,
+            tainted_by_errors: Cell::new(None),
+            forbidden_params: Default::default(),
+        }
     }
 
     pub fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
@@ -374,6 +385,58 @@ impl<'tcx> ItemCtxt<'tcx> {
             None => Ok(()),
         }
     }
+
+    fn forbid_unconstrained_lifetime_params_from_parent_impl(&self) -> Result<(), ErrorGuaranteed> {
+        let tcx = self.tcx;
+        let impl_def_id = tcx.hir().get_parent_item(self.hir_id());
+        let impl_self_ty = tcx.type_of(impl_def_id).instantiate_identity();
+        impl_self_ty.error_reported()?;
+        let impl_generics = tcx.generics_of(impl_def_id);
+        let impl_predicates = tcx.predicates_of(impl_def_id);
+        let impl_trait_ref =
+            tcx.impl_trait_ref(impl_def_id).map(ty::EarlyBinder::instantiate_identity);
+
+        impl_trait_ref.error_reported()?;
+
+        let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
+
+        cgp::identify_constrained_generic_params(
+            tcx,
+            impl_predicates,
+            impl_trait_ref,
+            &mut input_parameters,
+        );
+
+        for param in &impl_generics.own_params {
+            let p = match param.kind {
+                // This is a horrible concession to reality. It'd be
+                // better to just ban unconstrained lifetimes outright, but in
+                // practice people do non-hygienic macros like:
+                //
+                // ```
+                // macro_rules! __impl_slice_eq1 {
+                //     ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
+                //         impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
+                //            ....
+                //         }
+                //     }
+                // }
+                // ```
+                //
+                // In a concession to backwards compatibility, we continue to
+                // permit those, so long as the lifetimes aren't used in
+                // associated types. This is sound, because lifetimes
+                // used elsewhere are not projected back out.
+                ty::GenericParamDefKind::Type { .. } => continue,
+                ty::GenericParamDefKind::Lifetime => param.to_early_bound_region_data().into(),
+                ty::GenericParamDefKind::Const { .. } => continue,
+            };
+            if !input_parameters.contains(&p) {
+                self.forbidden_params.borrow_mut().insert(p, param.clone());
+            }
+        }
+        Ok(())
+    }
 }
 
 impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
@@ -515,8 +578,25 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
         ty.ty_adt_def()
     }
 
-    fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
-        // There's no place to record types from signatures?
+    fn record_ty(&self, _hir_id: hir::HirId, ty: Ty<'tcx>, span: Span) {
+        // There's no place to record types from signatures
+
+        if !self.forbidden_params.borrow().is_empty() {
+            for param in cgp::parameters_for(self.tcx, ty, true) {
+                if let Some(param) = self.forbidden_params.borrow_mut().remove(&param) {
+                    let mut diag = self.dcx().create_err(UnconstrainedGenericParameter {
+                        span: self.tcx.def_span(param.def_id),
+                        param_name: param.name,
+                        param_def_kind: self.tcx.def_descr(param.def_id),
+                        const_param_note: None,
+                        const_param_note2: None,
+                        lifetime_help: Some(span),
+                    });
+                    diag.code(E0207);
+                    diag.emit();
+                }
+            }
+        }
     }
 
     fn infcx(&self) -> Option<&InferCtxt<'tcx>> {

compiler/rustc_hir_analysis/src/collect/type_of.rs

@@ -341,6 +341,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
     use rustc_hir::*;
     use rustc_middle::ty::Ty;
 
+    let hir_id = tcx.local_def_id_to_hir_id(def_id);
+
+    let icx = ItemCtxt::new(tcx, def_id);
+
     // If we are computing `type_of` the synthesized associated type for an RPITIT in the impl
     // side, use `collect_return_position_impl_trait_in_trait_tys` to infer the value of the
     // associated type in the impl.
@@ -349,7 +353,15 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
             match tcx.collect_return_position_impl_trait_in_trait_tys(fn_def_id) {
                 Ok(map) => {
                     let assoc_item = tcx.associated_item(def_id);
-                    return map[&assoc_item.trait_item_def_id.unwrap()];
+                    let ty = map[&assoc_item.trait_item_def_id.unwrap()];
+                    match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
+                        Ok(()) => {
+                            // Ensure no unconstrained lifetimes are used in these impls.
+                            icx.record_ty(hir_id, ty.instantiate_identity(), tcx.def_span(def_id));
+                        }
+                        Err(guar) => return ty::EarlyBinder::bind(Ty::new_error(tcx, guar)),
+                    }
+                    return ty;
                 }
                 Err(_) => {
                     return ty::EarlyBinder::bind(Ty::new_error_with_message(
@@ -371,10 +383,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
         None => {}
     }
 
-    let hir_id = tcx.local_def_id_to_hir_id(def_id);
-
-    let icx = ItemCtxt::new(tcx, def_id);
-
     let output = match tcx.hir_node(hir_id) {
         Node::TraitItem(item) => match item.kind {
             TraitItemKind::Fn(..) => {
@@ -425,7 +433,10 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
                     check_feature_inherent_assoc_ty(tcx, item.span);
                 }
 
-                icx.lower_ty(ty)
+                match icx.forbid_unconstrained_lifetime_params_from_parent_impl() {
+                    Err(guar) => Ty::new_error(tcx, guar),
+                    Ok(()) => icx.lower_ty(ty),
+                }
             }
         },
 

compiler/rustc_hir_analysis/src/errors.rs

@@ -1616,6 +1616,8 @@ pub(crate) struct UnconstrainedGenericParameter {
     pub const_param_note: Option<()>,
     #[note(hir_analysis_const_param_note2)]
     pub const_param_note2: Option<()>,
+    #[help]
+    pub lifetime_help: Option<Span>,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_hir_analysis/src/impl_wf_check.rs

@@ -11,7 +11,6 @@
 use crate::{constrained_generic_params as cgp, errors::UnconstrainedGenericParameter};
 use min_specialization::check_min_specialization;
 
-use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::codes::*;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::LocalDefId;
@@ -83,25 +82,6 @@ fn enforce_impl_params_are_constrained(
         &mut input_parameters,
     );
 
-    // Disallow unconstrained lifetimes, but only if they appear in assoc types.
-    let lifetimes_in_associated_types: FxHashSet<_> = tcx
-        .associated_item_def_ids(impl_def_id)
-        .iter()
-        .flat_map(|def_id| {
-            let item = tcx.associated_item(def_id);
-            match item.kind {
-                ty::AssocKind::Type => {
-                    if item.defaultness(tcx).has_value() {
-                        cgp::parameters_for(tcx, tcx.type_of(def_id).instantiate_identity(), true)
-                    } else {
-                        vec![]
-                    }
-                }
-                ty::AssocKind::Fn | ty::AssocKind::Const => vec![],
-            }
-        })
-        .collect();
-
     let mut res = Ok(());
     for param in &impl_generics.own_params {
         let err = match param.kind {
@@ -110,11 +90,7 @@ fn enforce_impl_params_are_constrained(
                 let param_ty = ty::ParamTy::for_def(param);
                 !input_parameters.contains(&cgp::Parameter::from(param_ty))
             }
-            ty::GenericParamDefKind::Lifetime => {
-                let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
-                lifetimes_in_associated_types.contains(&param_lt) && // (*)
-                    !input_parameters.contains(&param_lt)
-            }
+            ty::GenericParamDefKind::Lifetime => false,
             ty::GenericParamDefKind::Const { .. } => {
                 let param_ct = ty::ParamConst::for_def(param);
                 !input_parameters.contains(&cgp::Parameter::from(param_ct))
@@ -129,29 +105,11 @@ fn enforce_impl_params_are_constrained(
                 param_def_kind: tcx.def_descr(param.def_id),
                 const_param_note,
                 const_param_note2: const_param_note,
+                lifetime_help: None,
             });
             diag.code(E0207);
             res = Err(diag.emit());
         }
     }
     res
-
-    // (*) This is a horrible concession to reality. I think it'd be
-    // better to just ban unconstrained lifetimes outright, but in
-    // practice people do non-hygienic macros like:
-    //
-    // ```
-    // macro_rules! __impl_slice_eq1 {
-    //     ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
-    //         impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
-    //            ....
-    //         }
-    //     }
-    // }
-    // ```
-    //
-    // In a concession to backwards compatibility, we continue to
-    // permit those, so long as the lifetimes aren't used in
-    // associated types. I believe this is sound, because lifetimes
-    // used elsewhere are not projected back out.
 }

tests/ui/associated-types/issue-26262.stderr

@@ -9,6 +9,12 @@ error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait,
    |
 LL | impl<'a,T: Trait2<'a>> Trait1<<T as Trait2<'a>>::Foo> for T {
    |      ^^ unconstrained lifetime parameter
+   |
+help: this use of an otherwise unconstrained lifetime is not allowed
+  --> $DIR/issue-26262.rs:19:16
+   |
+LL |     type Bar = &'a ();
+   |                ^^^^^^
 
 error: aborting due to 2 previous errors
 

tests/ui/async-await/in-trait/unconstrained-impl-region.rs

@@ -11,10 +11,11 @@ pub(crate) trait Actor: Sized {
 }
 
 impl<'a> Actor for () {
-//~^ ERROR the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
+    //~^ ERROR the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
     type Message = &'a ();
     async fn on_mount(self, _: impl Inbox<&'a ()>) {}
     //~^ ERROR the trait bound `impl Inbox<&'a ()>: Inbox<&'a ()>` is not satisfied
+    //~| ERROR impl has stricter requirements than trait
 }
 
 fn main() {}

tests/ui/async-await/in-trait/unconstrained-impl-region.stderr

@@ -1,3 +1,15 @@
+error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
+  --> $DIR/unconstrained-impl-region.rs:13:6
+   |
+LL | impl<'a> Actor for () {
+   |      ^^ unconstrained lifetime parameter
+   |
+help: this use of an otherwise unconstrained lifetime is not allowed
+  --> $DIR/unconstrained-impl-region.rs:15:20
+   |
+LL |     type Message = &'a ();
+   |                    ^^^^^^
+
 error[E0277]: the trait bound `impl Inbox<&'a ()>: Inbox<&'a ()>` is not satisfied
   --> $DIR/unconstrained-impl-region.rs:16:5
    |
@@ -10,13 +22,16 @@ note: required by a bound in `<() as Actor>::on_mount`
 LL |     async fn on_mount(self, _: impl Inbox<&'a ()>) {}
    |                                     ^^^^^^^^^^^^^ required by this bound in `<() as Actor>::on_mount`
 
-error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
-  --> $DIR/unconstrained-impl-region.rs:13:6
+error[E0276]: impl has stricter requirements than trait
+  --> $DIR/unconstrained-impl-region.rs:16:37
    |
-LL | impl<'a> Actor for () {
-   |      ^^ unconstrained lifetime parameter
+LL |     async fn on_mount(self, _: impl Inbox<Self::Message>);
+   |     ------------------------------------------------------ definition of `on_mount` from trait
+...
+LL |     async fn on_mount(self, _: impl Inbox<&'a ()>) {}
+   |                                     ^^^^^^^^^^^^^ impl has extra requirement `impl Inbox<&'a ()>: Inbox<&'a ()>`
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 
-Some errors have detailed explanations: E0207, E0277.
+Some errors have detailed explanations: E0207, E0276, E0277.
 For more information about an error, try `rustc --explain E0207`.

tests/ui/feature-gates/feature-gate-impl_trait_in_assoc_type.stderr

@@ -26,6 +26,14 @@ LL |     type Bar = impl std::fmt::Debug;
    |
    = note: `Bar` must be used in combination with a concrete type within the same impl
 
+error: unconstrained opaque type
+  --> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:16
+   |
+LL |     type Bop = impl std::fmt::Debug;
+   |                ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `Bop` must be used in combination with a concrete type within the same impl
+
 error[E0658]: inherent associated types are unstable
   --> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:5
    |
@@ -36,14 +44,6 @@ LL |     type Bop = impl std::fmt::Debug;
    = help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable
    = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
 
-error: unconstrained opaque type
-  --> $DIR/feature-gate-impl_trait_in_assoc_type.rs:14:16
-   |
-LL |     type Bop = impl std::fmt::Debug;
-   |                ^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `Bop` must be used in combination with a concrete type within the same impl
-
 error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0658`.