From d199a63b1160f3eaab372aa969ab91248ee24315 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 25 Oct 2024 10:55:38 +0200 Subject: [PATCH 1/3] ABI compatibility: remove section on target features --- library/core/src/primitive_docs.rs | 32 ++++++------------------------ 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 95fa6c9c950..f28b65c6f25 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1743,20 +1743,18 @@ mod prim_ref {} /// alignment, they might be passed in different registers and hence not be ABI-compatible. /// /// ABI compatibility as a concern only arises in code that alters the type of function pointers, -/// code that imports functions via `extern` blocks, and in code that combines `#[target_feature]` -/// with `extern fn`. Altering the type of function pointers is wildly unsafe (as in, a lot more -/// unsafe than even [`transmute_copy`][mem::transmute_copy]), and should only occur in the most -/// exceptional circumstances. Most Rust code just imports functions via `use`. `#[target_feature]` -/// is also used rarely. So, most likely you do not have to worry about ABI compatibility. +/// and code that imports functions via `extern` blocks. Altering the type of function pointers is +/// wildly unsafe (as in, a lot more unsafe than even [`transmute_copy`][mem::transmute_copy]), and +/// should only occur in the most exceptional circumstances. Most Rust code just imports functions +/// via `use`. So, most likely you do not have to worry about ABI compatibility. /// /// But assuming such circumstances, what are the rules? For this section, we are only considering /// the ABI of direct Rust-to-Rust calls, not linking in general -- once functions are imported via /// `extern` blocks, there are more things to consider that we do not go into here. /// /// For two signatures to be considered *ABI-compatible*, they must use a compatible ABI string, -/// must take the same number of arguments, the individual argument types and the return types must -/// be ABI-compatible, and the target feature requirements must be met (see the subsection below for -/// the last point). The ABI string is declared via `extern "ABI" fn(...) -> ...`; note that +/// must take the same number of arguments, and the individual argument types and the return types +/// must be ABI-compatible. The ABI string is declared via `extern "ABI" fn(...) -> ...`; note that /// `fn name(...) -> ...` implicitly uses the `"Rust"` ABI string and `extern fn name(...) -> ...` /// implicitly uses the `"C"` ABI string. /// @@ -1826,24 +1824,6 @@ mod prim_ref {} /// Behavior since transmuting `None::>` to `NonZero` violates the non-zero /// requirement. /// -/// #### Requirements concerning target features -/// -/// Under some conditions, the signature used by the caller and the callee can be ABI-incompatible -/// even if the exact same ABI string and types are being used. As an example, the -/// `std::arch::x86_64::__m256` type has a different `extern "C"` ABI when the `avx` feature is -/// enabled vs when it is not enabled. -/// -/// Therefore, to ensure ABI compatibility when code using different target features is combined -/// (such as via `#[target_feature]`), we further require that one of the following conditions is -/// met: -/// -/// - The function uses the `"Rust"` ABI string (which is the default without `extern`). -/// - Caller and callee are using the exact same set of target features. For the callee we consider -/// the features enabled (via `#[target_feature]` and `-C target-feature`/`-C target-cpu`) at the -/// declaration site; for the caller we consider the features enabled at the call site. -/// - Neither any argument nor the return value involves a SIMD type (`#[repr(simd)]`) that is not -/// behind a pointer indirection (i.e., `*mut __m256` is fine, but `(i32, __m256)` is not). -/// /// ### Trait implementations /// /// In this documentation the shorthand `fn(T₁, T₂, …, Tₙ)` is used to represent non-variadic From ad209060650deea966c9f272c75a7d41e51df4d7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 19:16:43 +0000 Subject: [PATCH 2/3] Suggest turning APITs into generics in opaque overcaptures --- compiler/rustc_lint/messages.ftl | 1 - .../rustc_lint/src/impl_trait_overcaptures.rs | 46 ++----- compiler/rustc_trait_selection/messages.ftl | 6 +- compiler/rustc_trait_selection/src/errors.rs | 124 +++++++++++++++++- .../precise-capturing/overcaptures-2024.fixed | 8 ++ .../precise-capturing/overcaptures-2024.rs | 8 ++ .../overcaptures-2024.stderr | 50 ++++++- 7 files changed, 202 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 9187f6caad4..6e35d89b488 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -346,7 +346,6 @@ lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than pos *[other] these lifetimes are } in scope but not mentioned in the type's bounds .note2 = all lifetimes in scope will be captured by `impl Trait`s in edition 2024 - .suggestion = use the precise capturing `use<...>` syntax to make the captures explicit lint_impl_trait_redundant_captures = all possible in-scope parameters are already captured, so `use<...>` syntax is redundant .suggestion = remove the `use<...>` syntax diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 002ab027982..62eaa51392b 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -3,7 +3,7 @@ use std::cell::LazyCell; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::unord::UnordSet; -use rustc_errors::{Applicability, LintDiagnostic}; +use rustc_errors::{LintDiagnostic, Subdiagnostic}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -22,6 +22,7 @@ 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::traits::ObligationCtxt; use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt; @@ -334,32 +335,12 @@ where // If we have uncaptured args, and if the opaque doesn't already have // `use<>` syntax on it, and we're < edition 2024, then warn the user. if !uncaptured_args.is_empty() { - let suggestion = if let Ok(snippet) = - self.tcx.sess.source_map().span_to_snippet(opaque_span) - && snippet.starts_with("impl ") - { - let (lifetimes, others): (Vec<_>, Vec<_>) = - captured.into_iter().partition(|def_id| { - self.tcx.def_kind(*def_id) == DefKind::LifetimeParam - }); - // Take all lifetime params first, then all others (ty/ct). - let generics: Vec<_> = lifetimes - .into_iter() - .chain(others) - .map(|def_id| self.tcx.item_name(def_id).to_string()) - .collect(); - // Make sure that we're not trying to name any APITs - if generics.iter().all(|name| !name.starts_with("impl ")) { - Some(( - format!(" + use<{}>", generics.join(", ")), - opaque_span.shrink_to_hi(), - )) - } else { - None - } - } else { - None - }; + let suggestion = impl_trait_overcapture_suggestion( + self.tcx, + opaque_def_id, + self.parent_def_id, + captured, + ); let uncaptured_spans: Vec<_> = uncaptured_args .into_iter() @@ -451,7 +432,7 @@ struct ImplTraitOvercapturesLint<'tcx> { uncaptured_spans: Vec, self_ty: Ty<'tcx>, num_captured: usize, - suggestion: Option<(String, Span)>, + suggestion: Option, } impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { @@ -461,13 +442,8 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { .arg("num_captured", self.num_captured) .span_note(self.uncaptured_spans, fluent::lint_note) .note(fluent::lint_note2); - if let Some((suggestion, span)) = self.suggestion { - diag.span_suggestion( - span, - fluent::lint_suggestion, - suggestion, - Applicability::MachineApplicable, - ); + if let Some(suggestion) = self.suggestion { + suggestion.add_to_diag(diag); } } } diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index 3ddd23924b5..6ca1db3a1de 100644 --- a/compiler/rustc_trait_selection/messages.ftl +++ b/compiler/rustc_trait_selection/messages.ftl @@ -280,6 +280,8 @@ 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_prlf_defined_with_sub = the lifetime `{$sub_symbol}` defined here... @@ -455,7 +457,9 @@ 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 = 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_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_where_copy_predicates = copy the `where` clause predicates from the trait diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs index 455e3ec751e..bd723cad511 100644 --- a/compiler/rustc_trait_selection/src/errors.rs +++ b/compiler/rustc_trait_selection/src/errors.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashSet, FxIndexSet}; use rustc_errors::codes::*; 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_id::LocalDefId; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{Visitor, walk_ty}; use rustc_hir::{FnRetTy, GenericParamKind}; use rustc_macros::{Diagnostic, Subdiagnostic}; @@ -1792,6 +1793,123 @@ impl Subdiagnostic for AddPreciseCapturingAndParams { self.suggs, Applicability::MaybeIncorrect, ); - diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params); + diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params_for_undercapture); } } + +pub fn impl_trait_overcapture_suggestion<'tcx>( + tcx: TyCtxt<'tcx>, + opaque_def_id: LocalDefId, + fn_def_id: LocalDefId, + captured_args: FxIndexSet, +) -> Option { + let generics = tcx.generics_of(fn_def_id); + + let mut captured_lifetimes = FxIndexSet::default(); + let mut captured_non_lifetimes = FxIndexSet::default(); + let mut synthetics = vec![]; + + for arg in captured_args { + if tcx.def_kind(arg) == DefKind::LifetimeParam { + captured_lifetimes.insert(tcx.item_name(arg)); + } else { + let idx = generics.param_def_id_to_index(tcx, arg).expect("expected arg in scope"); + let param = generics.param_at(idx as usize, tcx); + if param.kind.is_synthetic() { + synthetics.push((tcx.def_span(arg), param.name)); + } else { + captured_non_lifetimes.insert(tcx.item_name(arg)); + } + } + } + + let mut next_fresh_param = || { + ["T", "U", "V", "W", "X", "Y", "A", "B", "C"] + .into_iter() + .map(Symbol::intern) + .chain((0..).map(|i| Symbol::intern(&format!("T{i}")))) + .find(|s| captured_non_lifetimes.insert(*s)) + .unwrap() + }; + + let mut suggs = vec![]; + let mut apit_spans = vec![]; + + if !synthetics.is_empty() { + 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 + // the parameter, since APITs are always named after how they are + // rendered in the AST. This sucks! But to recreate the bound list + // from the APIT itself would be miserable, so we're stuck with + // this for now! + if i > 0 { + new_params += ", "; + } + let name_as_bounds = name.as_str().trim_start_matches("impl").trim_start(); + new_params += fresh_param.as_str(); + 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}")) + } else { + (generics.span, format!("<{new_params}>")) + }); + } + + let concatenated_bounds = captured_lifetimes + .into_iter() + .chain(captured_non_lifetimes) + .map(|sym| sym.to_string()) + .collect::>() + .join(", "); + + suggs.push(( + tcx.def_span(opaque_def_id).shrink_to_hi(), + format!(" + use<{concatenated_bounds}>"), + )); + + Some(AddPreciseCapturingForOvercapture { + suggs, + apit_spans, + }) +} + +pub struct AddPreciseCapturingForOvercapture { + pub suggs: Vec<(Span, String)>, + pub apit_spans: Vec, +} + +impl Subdiagnostic for AddPreciseCapturingForOvercapture { + fn add_to_diag_with>( + self, + diag: &mut Diag<'_, G>, + _f: &F, + ) { + diag.multipart_suggestion_verbose( + fluent::trait_selection_precise_capturing_overcaptures, + self.suggs, + Applicability::MaybeIncorrect, + ); + if !self.apit_spans.is_empty() { + 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/overcaptures-2024.fixed b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed index 89a3f3136c8..6a9d72d028c 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed @@ -29,4 +29,12 @@ fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized + use<>> {} //~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 //~| WARN this changes meaning in Rust 2024 +fn apit(_: &T) -> impl Sized + use {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + +fn apit2(_: &T, _: U) -> impl Sized + use {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs index 18c04f9f799..3a4f5ebb7fb 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs @@ -29,4 +29,12 @@ fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} //~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 //~| WARN this changes meaning in Rust 2024 +fn apit(_: &impl Sized) -> impl Sized {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + +fn apit2(_: &impl Sized, _: U) -> impl Sized {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 +//~| WARN this changes meaning in Rust 2024 + fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr index 94dafb04d64..c101b980c71 100644 --- a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr @@ -79,5 +79,53 @@ help: use the precise capturing `use<...>` syntax to make the captures explicit LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized + use<>> {} | +++++++ -error: aborting due to 4 previous errors +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:32:28 + | +LL | fn apit(_: &impl Sized) -> impl Sized {} + | ^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:32:12 + | +LL | fn apit(_: &impl Sized) -> impl Sized {} + | ^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +note: you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable + --> $DIR/overcaptures-2024.rs:32:13 + | +LL | fn apit(_: &impl Sized) -> impl Sized {} + | ^^^^^^^^^^ +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn apit(_: &T) -> impl Sized + use {} + | ++++++++++ ~ ++++++++ + +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:36:38 + | +LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} + | ^^^^^^^^^^ + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:36:16 + | +LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} + | ^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +note: you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable + --> $DIR/overcaptures-2024.rs:36:17 + | +LL | fn apit2(_: &impl Sized, _: U) -> impl Sized {} + | ^^^^^^^^^^ +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn apit2(_: &T, _: U) -> impl Sized + use {} + | ++++++++++ ~ +++++++++++ + +error: aborting due to 6 previous errors From a1f9d5bfbaa967ac91b862894d84e7b27b85d971 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 19:41:28 +0000 Subject: [PATCH 3/3] 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`.