Check use<..> in RPITIT for refinement

compiler/rustc_hir_analysis/messages.ftl

@@ -448,6 +448,11 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match
     .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
     .feedback_note = we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
 
+hir_analysis_rpitit_refined_lifetimes = impl trait in impl method captures fewer lifetimes than in trait
+    .suggestion = add a `use<..>` bound to capture the same lifetimes that the trait does
+    .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+    .feedback_note = we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
+
 hir_analysis_self_in_impl_self =
     `Self` is not valid in the self type of an impl block
     .note = replace `Self` with a different type

compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs

@@ -1,6 +1,7 @@
+use itertools::Itertools as _;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir as hir;
-use rustc_hir::def_id::DefId;
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_infer::infer::outlives::env::OutlivesEnvironment;
 use rustc_lint_defs::builtin::{REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE};
@@ -75,6 +76,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
     let mut trait_bounds = vec![];
     // Bounds that we find on the RPITITs in the impl signature.
     let mut impl_bounds = vec![];
+    // Pairs of trait and impl opaques.
+    let mut pairs = vec![];
 
     for trait_projection in collector.types.into_iter().rev() {
         let impl_opaque_args = trait_projection.args.rebase_onto(tcx, trait_m.def_id, impl_m_args);
@@ -121,6 +124,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
             tcx.explicit_item_bounds(impl_opaque.def_id)
                 .iter_instantiated_copied(tcx, impl_opaque.args),
         ));
+
+        pairs.push((trait_projection, impl_opaque));
     }
 
     let hybrid_preds = tcx
@@ -212,6 +217,39 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>(
             return;
         }
     }
+
+    // Make sure that the RPITIT doesn't capture fewer regions than
+    // the trait definition. We hard-error if it captures *more*, since that
+    // is literally unrepresentable in the type system; however, we may be
+    // promising stronger outlives guarantees if we capture *fewer* regions.
+    for (trait_projection, impl_opaque) in pairs {
+        let impl_variances = tcx.variances_of(impl_opaque.def_id);
+        let impl_captures: FxIndexSet<_> = impl_opaque
+            .args
+            .iter()
+            .zip_eq(impl_variances)
+            .filter(|(_, v)| **v == ty::Invariant)
+            .map(|(arg, _)| arg)
+            .collect();
+
+        let trait_variances = tcx.variances_of(trait_projection.def_id);
+        let mut trait_captures = FxIndexSet::default();
+        for (arg, variance) in trait_projection.args.iter().zip_eq(trait_variances) {
+            if *variance != ty::Invariant {
+                continue;
+            }
+            arg.visit_with(&mut CollectParams { params: &mut trait_captures });
+        }
+
+        if !trait_captures.iter().all(|arg| impl_captures.contains(arg)) {
+            report_mismatched_rpitit_captures(
+                tcx,
+                impl_opaque.def_id.expect_local(),
+                trait_captures,
+                is_internal,
+            );
+        }
+    }
 }
 
 struct ImplTraitInTraitCollector<'tcx> {
@@ -342,3 +380,65 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for Anonymize<'tcx> {
         self.tcx.anonymize_bound_vars(t)
     }
 }
+
+struct CollectParams<'a, 'tcx> {
+    params: &'a mut FxIndexSet<ty::GenericArg<'tcx>>,
+}
+impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for CollectParams<'_, 'tcx> {
+    fn visit_ty(&mut self, ty: Ty<'tcx>) {
+        if let ty::Param(_) = ty.kind() {
+            self.params.insert(ty.into());
+        } else {
+            ty.super_visit_with(self);
+        }
+    }
+    fn visit_region(&mut self, r: ty::Region<'tcx>) {
+        match r.kind() {
+            ty::ReEarlyParam(_) | ty::ReLateParam(_) => {
+                self.params.insert(r.into());
+            }
+            _ => {}
+        }
+    }
+    fn visit_const(&mut self, ct: ty::Const<'tcx>) {
+        if let ty::ConstKind::Param(_) = ct.kind() {
+            self.params.insert(ct.into());
+        } else {
+            ct.super_visit_with(self);
+        }
+    }
+}
+
+fn report_mismatched_rpitit_captures<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    impl_opaque_def_id: LocalDefId,
+    mut trait_captured_args: FxIndexSet<ty::GenericArg<'tcx>>,
+    is_internal: bool,
+) {
+    let Some(use_bound_span) =
+        tcx.hir_node_by_def_id(impl_opaque_def_id).expect_opaque_ty().bounds.iter().find_map(
+            |bound| match *bound {
+                rustc_hir::GenericBound::Use(_, span) => Some(span),
+                hir::GenericBound::Trait(_) | hir::GenericBound::Outlives(_) => None,
+            },
+        )
+    else {
+        // I have no idea when you would ever undercapture without a `use<..>`.
+        tcx.dcx().delayed_bug("expected use<..> to undercapture in an impl opaque");
+        return;
+    };
+
+    trait_captured_args
+        .sort_by_cached_key(|arg| !matches!(arg.unpack(), ty::GenericArgKind::Lifetime(_)));
+    let suggestion = format!("use<{}>", trait_captured_args.iter().join(", "));
+
+    tcx.emit_node_span_lint(
+        if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE },
+        tcx.local_def_id_to_hir_id(impl_opaque_def_id),
+        use_bound_span,
+        crate::errors::ReturnPositionImplTraitInTraitRefinedLifetimes {
+            suggestion_span: use_bound_span,
+            suggestion,
+        },
+    );
+}

compiler/rustc_hir_analysis/src/errors.rs

@@ -1153,6 +1153,16 @@ pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> {
     pub return_ty: Ty<'tcx>,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(hir_analysis_rpitit_refined_lifetimes)]
+#[note]
+#[note(hir_analysis_feedback_note)]
+pub(crate) struct ReturnPositionImplTraitInTraitRefinedLifetimes {
+    #[suggestion(applicability = "maybe-incorrect", code = "{suggestion}")]
+    pub suggestion_span: Span,
+    pub suggestion: String,
+}
+
 #[derive(Diagnostic)]
 #[diag(hir_analysis_inherent_ty_outside, code = E0390)]
 #[help]

tests/ui/impl-trait/in-trait/refine-captures.rs

@@ -0,0 +1,36 @@
+#![feature(precise_capturing_in_traits)]
+
+trait LifetimeParam<'a> {
+    fn test() -> impl Sized;
+}
+// Refining via capturing fewer lifetimes than the trait definition.
+impl<'a> LifetimeParam<'a> for i32 {
+    fn test() -> impl Sized + use<> {}
+    //~^ WARN impl trait in impl method captures fewer lifetimes than in trait
+}
+// If the lifetime is substituted, then we don't refine anything.
+impl LifetimeParam<'static> for u32 {
+    fn test() -> impl Sized + use<> {}
+    // Ok
+}
+
+trait TypeParam<T> {
+    fn test() -> impl Sized;
+}
+// Indirectly capturing a lifetime param through a type param substitution.
+impl<'a> TypeParam<&'a ()> for i32 {
+    fn test() -> impl Sized + use<> {}
+    //~^ WARN impl trait in impl method captures fewer lifetimes than in trait
+}
+// Two of them, but only one is captured...
+impl<'a, 'b> TypeParam<(&'a (), &'b ())> for u32 {
+    fn test() -> impl Sized + use<'b> {}
+    //~^ WARN impl trait in impl method captures fewer lifetimes than in trait
+}
+// What if we don't capture a type param? That should be an error otherwise.
+impl<T> TypeParam<T> for u64 {
+    fn test() -> impl Sized + use<> {}
+    //~^ ERROR `impl Trait` must mention all type parameters in scope in `use<...>`
+}
+
+fn main() {}

tests/ui/impl-trait/in-trait/refine-captures.stderr

@@ -0,0 +1,52 @@
+warning: impl trait in impl method captures fewer lifetimes than in trait
+  --> $DIR/refine-captures.rs:8:31
+   |
+LL |     fn test() -> impl Sized + use<> {}
+   |                               ^^^^^
+   |
+   = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+   = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
+   = note: `#[warn(refining_impl_trait_internal)]` on by default
+help: add a `use<..>` bound to capture the same lifetimes that the trait does
+   |
+LL |     fn test() -> impl Sized + use<'a> {}
+   |                               ~~~~~~~
+
+warning: impl trait in impl method captures fewer lifetimes than in trait
+  --> $DIR/refine-captures.rs:22:31
+   |
+LL |     fn test() -> impl Sized + use<> {}
+   |                               ^^^^^
+   |
+   = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+   = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
+help: add a `use<..>` bound to capture the same lifetimes that the trait does
+   |
+LL |     fn test() -> impl Sized + use<'a> {}
+   |                               ~~~~~~~
+
+warning: impl trait in impl method captures fewer lifetimes than in trait
+  --> $DIR/refine-captures.rs:27:31
+   |
+LL |     fn test() -> impl Sized + use<'b> {}
+   |                               ^^^^^^^
+   |
+   = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate
+   = note: we are soliciting feedback, see issue #121718 <https://github.com/rust-lang/rust/issues/121718> for more information
+help: add a `use<..>` bound to capture the same lifetimes that the trait does
+   |
+LL |     fn test() -> impl Sized + use<'a, 'b> {}
+   |                               ~~~~~~~~~~~
+
+error: `impl Trait` must mention all type parameters in scope in `use<...>`
+  --> $DIR/refine-captures.rs:32:18
+   |
+LL | impl<T> TypeParam<T> for u64 {
+   |      - type parameter is implicitly captured by this `impl Trait`
+LL |     fn test() -> impl Sized + use<> {}
+   |                  ^^^^^^^^^^^^^^^^^^
+   |
+   = note: currently, all type parameters are required to be mentioned in the precise captures list
+
+error: aborting due to 1 previous error; 3 warnings emitted
+