Skip to content

Commit

Permalink
Unrolled build for rust-lang#132795
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#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:
* rust-lang#130044
  • Loading branch information
rust-timer authored Nov 18, 2024
2 parents 3fb7e44 + 32d2340 commit 0f3042b
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 1 deletion.
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

0 comments on commit 0f3042b

Please sign in to comment.