From ad209060650deea966c9f272c75a7d41e51df4d7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 19:16:43 +0000 Subject: [PATCH] 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 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::{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 @@ fn visit_ty(&mut self, t: Ty<'tcx>) { // 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 @@ fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) { .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 @@ fn add_to_diag_with>( 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