Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check use<..> in RPITIT for refinement #132795

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
102 changes: 101 additions & 1 deletion compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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,
},
);
}
10 changes: 10 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/impl-trait/in-trait/refine-captures.rs
Original file line number Diff line number Diff line change
@@ -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() {}
52 changes: 52 additions & 0 deletions tests/ui/impl-trait/in-trait/refine-captures.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading