From a1f9d5bfbaa967ac91b862894d84e7b27b85d971 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 19:41:28 +0000 Subject: [PATCH] Dont suggest use --- .../src/diagnostics/opaque_suggestions.rs | 71 ++++++++++++------- .../rustc_lint/src/impl_trait_overcaptures.rs | 4 +- compiler/rustc_trait_selection/messages.ftl | 8 +-- compiler/rustc_trait_selection/src/errors.rs | 33 +++++---- .../precise-capturing/migration-note.rs | 15 ++++ .../precise-capturing/migration-note.stderr | 58 +++++++++++---- 6 files changed, 133 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs b/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs index bfd7e83501c..d77c53a3984 100644 --- a/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs +++ b/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs @@ -4,15 +4,16 @@ use std::ops::ControlFlow; use either::Either; +use itertools::Itertools as _; use rustc_data_structures::fx::FxIndexSet; -use rustc_errors::{Applicability, Diag}; +use rustc_errors::{Diag, Subdiagnostic}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::mir::{self, ConstraintCategory, Location}; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; -use rustc_span::Symbol; +use rustc_trait_selection::errors::impl_trait_overcapture_suggestion; use crate::MirBorrowckCtxt; use crate::borrow_set::BorrowData; @@ -61,6 +62,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // *does* mention. We'll use that for the `+ use<'a>` suggestion below. let mut visitor = CheckExplicitRegionMentionAndCollectGenerics { tcx, + generics: tcx.generics_of(opaque_def_id), offending_region_idx, seen_opaques: [opaque_def_id].into_iter().collect(), seen_lifetimes: Default::default(), @@ -83,34 +85,50 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { "this call may capture more lifetimes than intended, \ because Rust 2024 has adjusted the `impl Trait` lifetime capture rules", ); - let mut seen_generics: Vec<_> = - visitor.seen_lifetimes.iter().map(ToString::to_string).collect(); - // Capture all in-scope ty/const params. - seen_generics.extend( - ty::GenericArgs::identity_for_item(tcx, opaque_def_id) - .iter() - .filter(|arg| { - matches!( - arg.unpack(), - ty::GenericArgKind::Type(_) | ty::GenericArgKind::Const(_) - ) - }) - .map(|arg| arg.to_string()), - ); - if opaque_def_id.is_local() { - diag.span_suggestion_verbose( - tcx.def_span(opaque_def_id).shrink_to_hi(), - "add a precise capturing bound to avoid overcapturing", - format!(" + use<{}>", seen_generics.join(", ")), - Applicability::MaybeIncorrect, - ); + let mut captured_args = visitor.seen_lifetimes; + // Add in all of the type and const params, too. + // Ordering here is kinda strange b/c we're walking backwards, + // but we're trying to provide *a* suggestion, not a nice one. + let mut next_generics = Some(visitor.generics); + let mut any_synthetic = false; + while let Some(generics) = next_generics { + for param in &generics.own_params { + if param.kind.is_ty_or_const() { + captured_args.insert(param.def_id); + } + if param.kind.is_synthetic() { + any_synthetic = true; + } + } + next_generics = generics.parent.map(|def_id| tcx.generics_of(def_id)); + } + + if let Some(opaque_def_id) = opaque_def_id.as_local() + && let hir::OpaqueTyOrigin::FnReturn { parent, .. } = + tcx.hir().expect_opaque_ty(opaque_def_id).origin + { + if let Some(sugg) = impl_trait_overcapture_suggestion( + tcx, + opaque_def_id, + parent, + captured_args, + ) { + sugg.add_to_diag(diag); + } } else { diag.span_help( tcx.def_span(opaque_def_id), format!( "if you can modify this crate, add a precise \ capturing bound to avoid overcapturing: `+ use<{}>`", - seen_generics.join(", ") + if any_synthetic { + "/* Args */".to_string() + } else { + captured_args + .into_iter() + .map(|def_id| tcx.item_name(def_id)) + .join(", ") + } ), ); } @@ -182,9 +200,10 @@ impl<'tcx> TypeVisitor> for FindOpaqueRegion<'_, 'tcx> { struct CheckExplicitRegionMentionAndCollectGenerics<'tcx> { tcx: TyCtxt<'tcx>, + generics: &'tcx ty::Generics, offending_region_idx: usize, seen_opaques: FxIndexSet, - seen_lifetimes: FxIndexSet, + seen_lifetimes: FxIndexSet, } impl<'tcx> TypeVisitor> for CheckExplicitRegionMentionAndCollectGenerics<'tcx> { @@ -214,7 +233,7 @@ impl<'tcx> TypeVisitor> for CheckExplicitRegionMentionAndCollectGen if param.index as usize == self.offending_region_idx { ControlFlow::Break(()) } else { - self.seen_lifetimes.insert(param.name); + self.seen_lifetimes.insert(self.generics.region_param(param, self.tcx).def_id); ControlFlow::Continue(()) } } diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 62eaa51392b..036e0381a06 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -22,7 +22,9 @@ use rustc_session::lint::FutureIncompatibilityReason; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::edition::Edition; use rustc_span::{Span, Symbol}; -use rustc_trait_selection::errors::{impl_trait_overcapture_suggestion, AddPreciseCapturingForOvercapture}; +use rustc_trait_selection::errors::{ + AddPreciseCapturingForOvercapture, impl_trait_overcapture_suggestion, +}; use rustc_trait_selection::traits::ObligationCtxt; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt; diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index 6ca1db3a1de..1ab89ecde7a 100644 --- a/compiler/rustc_trait_selection/messages.ftl +++ b/compiler/rustc_trait_selection/messages.ftl @@ -280,10 +280,10 @@ trait_selection_outlives_content = lifetime of reference outlives lifetime of bo trait_selection_precise_capturing_existing = add `{$new_lifetime}` to the `use<...>` bound to explicitly capture it trait_selection_precise_capturing_new = add a `use<...>` bound to explicitly capture `{$new_lifetime}` -trait_selection_precise_capturing_overcaptures = use the precise capturing `use<...>` syntax to make the captures explicit - trait_selection_precise_capturing_new_but_apit = add a `use<...>` bound to explicitly capture `{$new_lifetime}` after turning all argument-position `impl Trait` into type parameters, noting that this possibly affects the API of this crate +trait_selection_precise_capturing_overcaptures = use the precise capturing `use<...>` syntax to make the captures explicit + trait_selection_prlf_defined_with_sub = the lifetime `{$sub_symbol}` defined here... trait_selection_prlf_defined_without_sub = the lifetime defined here... trait_selection_prlf_known_limitation = this is a known limitation that will be removed in the future (see issue #100013 for more information) @@ -457,10 +457,10 @@ trait_selection_unable_to_construct_constant_value = unable to construct a const trait_selection_unknown_format_parameter_for_on_unimplemented_attr = there is no parameter `{$argument_name}` on trait `{$trait_name}` .help = expect either a generic argument name or {"`{Self}`"} as format argument -trait_selection_warn_removing_apit_params_for_undercapture = you could use a `use<...>` bound to explicitly capture `{$new_lifetime}`, but argument-position `impl Trait`s are not nameable - trait_selection_warn_removing_apit_params_for_overcapture = you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable +trait_selection_warn_removing_apit_params_for_undercapture = you could use a `use<...>` bound to explicitly capture `{$new_lifetime}`, but argument-position `impl Trait`s are not nameable + trait_selection_where_copy_predicates = copy the `where` clause predicates from the trait trait_selection_where_remove = remove the `where` clause diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs index bd723cad511..3e06d0807d8 100644 --- a/compiler/rustc_trait_selection/src/errors.rs +++ b/compiler/rustc_trait_selection/src/errors.rs @@ -6,8 +6,8 @@ use rustc_errors::{ Applicability, Diag, DiagCtxtHandle, DiagMessage, DiagStyledString, Diagnostic, EmissionGuarantee, IntoDiagArg, Level, MultiSpan, SubdiagMessageOp, Subdiagnostic, }; -use rustc_hir::def::DefKind; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{Visitor, walk_ty}; use rustc_hir::{FnRetTy, GenericParamKind}; @@ -1793,10 +1793,17 @@ impl Subdiagnostic for AddPreciseCapturingAndParams { self.suggs, Applicability::MaybeIncorrect, ); - diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params_for_undercapture); + diag.span_note( + self.apit_spans, + fluent::trait_selection_warn_removing_apit_params_for_undercapture, + ); } } +/// Given a set of captured `DefId` for an RPIT (opaque_def_id) and a given +/// function (fn_def_id), try to suggest adding `+ use<...>` to capture just +/// the specified parameters. If one of those parameters is an APIT, then try +/// to suggest turning it into a regular type parameter. pub fn impl_trait_overcapture_suggestion<'tcx>( tcx: TyCtxt<'tcx>, opaque_def_id: LocalDefId, @@ -1839,12 +1846,12 @@ pub fn impl_trait_overcapture_suggestion<'tcx>( let mut new_params = String::new(); for (i, (span, name)) in synthetics.into_iter().enumerate() { apit_spans.push(span); - + let fresh_param = next_fresh_param(); - + // Suggest renaming. suggs.push((span, fresh_param.to_string())); - + // Super jank. Turn `impl Trait` into `T: Trait`. // // This currently involves stripping the `impl` from the name of @@ -1860,12 +1867,12 @@ pub fn impl_trait_overcapture_suggestion<'tcx>( new_params += ": "; new_params += name_as_bounds; } - + let Some(generics) = tcx.hir().get_generics(fn_def_id) else { // This shouldn't happen, but don't ICE. return None; }; - + // Add generics or concatenate to the end of the list. suggs.push(if let Some(params_span) = generics.span_for_param_suggestion() { (params_span, format!(", {new_params}")) @@ -1886,10 +1893,7 @@ pub fn impl_trait_overcapture_suggestion<'tcx>( format!(" + use<{concatenated_bounds}>"), )); - Some(AddPreciseCapturingForOvercapture { - suggs, - apit_spans, - }) + Some(AddPreciseCapturingForOvercapture { suggs, apit_spans }) } pub struct AddPreciseCapturingForOvercapture { @@ -1909,7 +1913,10 @@ impl Subdiagnostic for AddPreciseCapturingForOvercapture { Applicability::MaybeIncorrect, ); if !self.apit_spans.is_empty() { - diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params_for_overcapture); + diag.span_note( + self.apit_spans, + fluent::trait_selection_warn_removing_apit_params_for_overcapture, + ); } } -} \ No newline at end of file +} diff --git a/tests/ui/impl-trait/precise-capturing/migration-note.rs b/tests/ui/impl-trait/precise-capturing/migration-note.rs index a5bade4ddc5..1d98750f6dd 100644 --- a/tests/ui/impl-trait/precise-capturing/migration-note.rs +++ b/tests/ui/impl-trait/precise-capturing/migration-note.rs @@ -187,4 +187,19 @@ fn returned() -> impl Sized { } //~^ NOTE `x` dropped here while still borrowed +fn capture_apit(x: &impl Sized) -> impl Sized {} +//~^ NOTE you could use a `use<...>` bound to explicitly specify captures, but + +fn test_apit() { + let x = String::new(); + //~^ NOTE binding `x` declared here + let y = capture_apit(&x); + //~^ NOTE borrow of `x` occurs here + //~| NOTE this call may capture more lifetimes than intended + drop(x); + //~^ ERROR cannot move out of `x` because it is borrowed + //~| NOTE move out of `x` occurs here +} +//~^ NOTE borrow might be used here, when `y` is dropped + fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/migration-note.stderr b/tests/ui/impl-trait/precise-capturing/migration-note.stderr index 3ac47ed1bcd..a859a114dbc 100644 --- a/tests/ui/impl-trait/precise-capturing/migration-note.stderr +++ b/tests/ui/impl-trait/precise-capturing/migration-note.stderr @@ -30,7 +30,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len(&x); | ^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len(x: &Vec) -> impl Display + use { | ++++++++ @@ -55,7 +55,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len(&x); | ^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len(x: &Vec) -> impl Display + use { | ++++++++ @@ -80,7 +80,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len(&x); | ^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len(x: &Vec) -> impl Display + use { | ++++++++ @@ -106,7 +106,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len_mut(&mut x); | ^^^^^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len_mut(x: &mut Vec) -> impl Display + use { | ++++++++ @@ -131,7 +131,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len_mut(&mut x); | ^^^^^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len_mut(x: &mut Vec) -> impl Display + use { | ++++++++ @@ -156,7 +156,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_len_mut(&mut x); | ^^^^^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len_mut(x: &mut Vec) -> impl Display + use { | ++++++++ @@ -182,7 +182,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_field(&s.f); | ^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_field(t: &T) -> impl Display + use { | ++++++++ @@ -204,7 +204,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_field(&mut s.f); | ^^^^^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_field(t: &T) -> impl Display + use { | ++++++++ @@ -226,7 +226,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | let a = display_field(&mut s.f); | ^^^^^^^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_field(t: &T) -> impl Display + use { | ++++++++ @@ -252,7 +252,7 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has | LL | x = display_len(&z.f); | ^^^^^^^^^^^^^^^^^ -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len(x: &Vec) -> impl Display + use { | ++++++++ @@ -273,12 +273,46 @@ note: this call may capture more lifetimes than intended, because Rust 2024 has LL | let x = { let x = display_len(&mut vec![0]); x }; | ^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info) -help: add a precise capturing bound to avoid overcapturing +help: use the precise capturing `use<...>` syntax to make the captures explicit | LL | fn display_len(x: &Vec) -> impl Display + use { | ++++++++ -error: aborting due to 12 previous errors +error[E0505]: cannot move out of `x` because it is borrowed + --> $DIR/migration-note.rs:199:10 + | +LL | let x = String::new(); + | - binding `x` declared here +LL | +LL | let y = capture_apit(&x); + | -- borrow of `x` occurs here +... +LL | drop(x); + | ^ move out of `x` occurs here +... +LL | } + | - borrow might be used here, when `y` is dropped and runs the destructor for type `impl Sized` + | +note: this call may capture more lifetimes than intended, because Rust 2024 has adjusted the `impl Trait` lifetime capture rules + --> $DIR/migration-note.rs:196:13 + | +LL | let y = capture_apit(&x); + | ^^^^^^^^^^^^^^^^ +note: you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable + --> $DIR/migration-note.rs:190:21 + | +LL | fn capture_apit(x: &impl Sized) -> impl Sized {} + | ^^^^^^^^^^ +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn capture_apit(x: &T) -> impl Sized + use {} + | ++++++++++ ~ ++++++++ +help: consider cloning the value if the performance cost is acceptable + | +LL | let y = capture_apit(&x.clone()); + | ++++++++ + +error: aborting due to 13 previous errors Some errors have detailed explanations: E0499, E0502, E0503, E0505, E0506, E0597, E0716. For more information about an error, try `rustc --explain E0499`.