Rollup merge of #132795 - compiler-errors:refine-rpitit, r=lcnr

Check `use<..>` in RPITIT for refinement

`#![feature(precise_capturing_in_traits)]` allows users to write `+ use<>` bounds on RPITITs to control what lifetimes are captured by the RPITIT.

Since RPITITs currently also warn for refinement in implementations, this PR extends that refinement check for cases where we *undercapture* in an implementation, since that may be indirectly "promising" a more relaxed outlives bound than the impl author intended.

For an opaque to be refining, we need to capture *fewer* parameters than those mentioned in the captured params of the trait. For example:

```
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
}
```

Since the opaque in the method (implicitly) captures `use<Self, T>`, and `Self = i32, T = &'a ()` in the impl, we must mention `'a` in our `use<..>` on the impl.

Tracking:
* https://github.com/rust-lang/rust/issues/130044

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 = modify the `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: modify the `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: modify the `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: modify the `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
+