about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJacob Pratt <jacob@jhpratt.dev>2024-11-17 22:30:47 -0500
committerGitHub <noreply@github.com>2024-11-17 22:30:47 -0500
commite2993cd06e7b35422fcf172b0ff247873ddbb163 (patch)
treed19df3d6611a70af5a0dc883464af87fb407e47a
parent3fb7e441aecc3c054d71eb4d752d06e7776e8888 (diff)
parent32d2340dbd4e9e724839c5ee8c6e73474660fe89 (diff)
downloadrust-e2993cd06e7b35422fcf172b0ff247873ddbb163.tar.gz
rust-e2993cd06e7b35422fcf172b0ff247873ddbb163.zip
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
-rw-r--r--compiler/rustc_hir_analysis/messages.ftl5
-rw-r--r--compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs102
-rw-r--r--compiler/rustc_hir_analysis/src/errors.rs10
-rw-r--r--tests/ui/impl-trait/in-trait/refine-captures.rs36
-rw-r--r--tests/ui/impl-trait/in-trait/refine-captures.stderr52
5 files changed, 204 insertions, 1 deletions
diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl
index 6e8ba51612e..64a30e633cf 100644
--- a/compiler/rustc_hir_analysis/messages.ftl
+++ b/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
diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs
index 646c104f1f5..25ba52b4d7b 100644
--- a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs
+++ b/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,
+        },
+    );
+}
diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs
index a92a5e4278c..07d3273b09c 100644
--- a/compiler/rustc_hir_analysis/src/errors.rs
+++ b/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]
diff --git a/tests/ui/impl-trait/in-trait/refine-captures.rs b/tests/ui/impl-trait/in-trait/refine-captures.rs
new file mode 100644
index 00000000000..e7dffcb52aa
--- /dev/null
+++ b/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() {}
diff --git a/tests/ui/impl-trait/in-trait/refine-captures.stderr b/tests/ui/impl-trait/in-trait/refine-captures.stderr
new file mode 100644
index 00000000000..ad2c2a11601
--- /dev/null
+++ b/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
+