From 32d2340dbd4e9e724839c5ee8c6e73474660fe89 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 00:59:35 +0000 Subject: [PATCH] Check use<..> in RPITIT for refinement --- compiler/rustc_hir_analysis/messages.ftl | 5 + .../src/check/compare_impl_item/refine.rs | 102 +++++++++++++++++- compiler/rustc_hir_analysis/src/errors.rs | 10 ++ .../ui/impl-trait/in-trait/refine-captures.rs | 36 +++++++ .../in-trait/refine-captures.stderr | 52 +++++++++ 5 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 tests/ui/impl-trait/in-trait/refine-captures.rs create mode 100644 tests/ui/impl-trait/in-trait/refine-captures.stderr 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 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 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> for Anonymize<'tcx> { self.tcx.anonymize_bound_vars(t) } } + +struct CollectParams<'a, 'tcx> { + params: &'a mut FxIndexSet>, +} +impl<'tcx> TypeVisitor> 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>, + 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 { + 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 TypeParam 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 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 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 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 TypeParam 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 +