From d57e57ca1ff5abe96aaab94913c5c81c6027f39c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Apr 2024 21:13:35 -0400 Subject: [PATCH 1/6] Implement initial IMPL_TRAIT_OVERCAPTURES lint --- compiler/rustc_lint/messages.ftl | 7 + .../rustc_lint/src/impl_trait_overcaptures.rs | 236 ++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + 3 files changed, 246 insertions(+) create mode 100644 compiler/rustc_lint/src/impl_trait_overcaptures.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 676a7c21841..67b2121e5c0 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -9,6 +9,13 @@ lint_array_into_iter = .use_explicit_into_iter_suggestion = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value +lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024 + .note = specifically, {$num_captured -> + [one] this lifetime is + *[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 + lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future` .suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs new file mode 100644 index 00000000000..a633f355f29 --- /dev/null +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -0,0 +1,236 @@ +use rustc_data_structures::{fx::FxIndexSet, unord::UnordSet}; +use rustc_errors::LintDiagnostic; +use rustc_hir as hir; +use rustc_hir::def::DefKind; +use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::intravisit; +use rustc_middle::ty::{ + self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, +}; +use rustc_span::Span; + +use crate::fluent_generated as fluent; +use crate::{LateContext, LateLintPass}; + +declare_lint! { + /// UwU + pub IMPL_TRAIT_OVERCAPTURES, + Warn, + "will capture more lifetimes than possibly intended in edition 2024", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), + reference: "", + }; +} + +declare_lint_pass!( + /// Lint for use of `async fn` in the definition of a publicly-reachable + /// trait. + ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES] +); + +impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: intravisit::FnKind<'tcx>, + _: &'tcx hir::FnDecl<'tcx>, + _: &'tcx hir::Body<'tcx>, + _: Span, + parent_def_id: LocalDefId, + ) { + match cx.tcx.def_kind(parent_def_id) { + DefKind::AssocFn => { + // RPITITs already capture all lifetimes in scope, so skip them. + if matches!( + cx.tcx.def_kind(cx.tcx.local_parent(parent_def_id)), + DefKind::Trait | DefKind::Impl { of_trait: true } + ) { + return; + } + } + DefKind::Fn => { + // All freee functions need to check for overcaptures. + } + DefKind::Closure => return, + kind => { + unreachable!( + "expected function item, found {}", + kind.descr(parent_def_id.to_def_id()) + ) + } + } + + let sig = cx.tcx.fn_sig(parent_def_id).instantiate_identity(); + + let mut in_scope_parameters = FxIndexSet::default(); + let mut current_def_id = Some(parent_def_id.to_def_id()); + while let Some(def_id) = current_def_id { + let generics = cx.tcx.generics_of(def_id); + for param in &generics.params { + in_scope_parameters.insert(param.def_id); + } + current_def_id = generics.parent; + } + + sig.visit_with(&mut VisitOpaqueTypes { + tcx: cx.tcx, + parent_def_id, + in_scope_parameters, + seen: Default::default(), + }); + } +} + +struct VisitOpaqueTypes<'tcx> { + tcx: TyCtxt<'tcx>, + parent_def_id: LocalDefId, + in_scope_parameters: FxIndexSet, + seen: FxIndexSet, +} + +impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { + fn visit_binder>>( + &mut self, + t: &ty::Binder<'tcx, T>, + ) -> Self::Result { + let mut added = vec![]; + for arg in t.bound_vars() { + let arg: ty::BoundVariableKind = arg; + match arg { + ty::BoundVariableKind::Region(ty::BoundRegionKind::BrNamed(def_id, ..)) => { + added.push(def_id); + let unique = self.in_scope_parameters.insert(def_id); + assert!(unique); + } + ty::BoundVariableKind::Ty(_) => { + todo!("we don't support late-bound type params in `impl Trait`") + } + ty::BoundVariableKind::Region(..) => { + unreachable!("all AST-derived bound regions should have a name") + } + ty::BoundVariableKind::Const => { + unreachable!("non-lifetime binder consts are not allowed") + } + } + } + + t.super_visit_with(self); + + for arg in added.into_iter().rev() { + self.in_scope_parameters.shift_remove(&arg); + } + } + + fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { + if !t.has_opaque_types() { + return; + } + + if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() + && let Some(opaque_def_id) = opaque_ty.def_id.as_local() + && self.seen.insert(opaque_def_id) + && let opaque = + self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty() + && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin + && parent_def_id == self.parent_def_id + && opaque.precise_capturing_args.is_none() + { + let mut captured = UnordSet::default(); + let variances = self.tcx.variances_of(opaque_def_id); + let mut current_def_id = Some(opaque_def_id.to_def_id()); + while let Some(def_id) = current_def_id { + let generics = self.tcx.generics_of(def_id); + for param in &generics.params { + if variances[param.index as usize] != ty::Invariant { + continue; + } + captured.insert(extract_def_id_from_arg( + self.tcx, + generics, + opaque_ty.args[param.index as usize], + )); + } + current_def_id = generics.parent; + } + + let uncaptured_spans: Vec<_> = self + .in_scope_parameters + .iter() + .filter(|def_id| !captured.contains(def_id)) + .map(|def_id| self.tcx.def_span(def_id)) + .collect(); + + if !uncaptured_spans.is_empty() { + self.tcx.emit_node_lint( + IMPL_TRAIT_OVERCAPTURES, + self.tcx.local_def_id_to_hir_id(opaque_def_id), + ImplTraitOvercapturesLint { + opaque_span: self.tcx.def_span(opaque_def_id), + self_ty: t, + num_captured: uncaptured_spans.len(), + uncaptured_spans, + }, + ); + } + + for clause in + self.tcx.item_bounds(opaque_ty.def_id).iter_instantiated(self.tcx, opaque_ty.args) + { + clause.visit_with(self) + } + } + + t.super_visit_with(self); + } +} + +struct ImplTraitOvercapturesLint<'tcx> { + opaque_span: Span, + uncaptured_spans: Vec, + self_ty: Ty<'tcx>, + num_captured: usize, +} + +impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { + fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) { + diag.arg("self_ty", self.self_ty.to_string()) + .arg("num_captured", self.num_captured) + .span(self.opaque_span) + .span_note(self.uncaptured_spans, fluent::lint_note) + .note(fluent::lint_note2); + } + + fn msg(&self) -> rustc_errors::DiagMessage { + fluent::lint_impl_trait_overcaptures + } +} + +fn extract_def_id_from_arg<'tcx>( + tcx: TyCtxt<'tcx>, + generics: &'tcx ty::Generics, + arg: ty::GenericArg<'tcx>, +) -> DefId { + match arg.unpack() { + ty::GenericArgKind::Lifetime(re) => match *re { + ty::ReEarlyParam(ebr) => generics.region_param(ebr, tcx).def_id, + ty::ReBound( + _, + ty::BoundRegion { kind: ty::BoundRegionKind::BrNamed(def_id, ..), .. }, + ) => def_id, + _ => unreachable!(), + }, + ty::GenericArgKind::Type(ty) => { + let ty::Param(param_ty) = *ty.kind() else { + bug!(); + }; + generics.type_param(param_ty, tcx).def_id + } + ty::GenericArgKind::Const(ct) => { + let ty::ConstKind::Param(param_ct) = ct.kind() else { + bug!(); + }; + generics.const_param(param_ct, tcx).def_id + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a78b410f500..d93edadcfbc 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -55,6 +55,7 @@ mod for_loops_over_fallibles; mod foreign_modules; pub mod hidden_unicode_codepoints; +mod impl_trait_overcaptures; mod internal; mod invalid_from_utf8; mod late; @@ -94,6 +95,7 @@ use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; +use impl_trait_overcaptures::ImplTraitOvercaptures; use internal::*; use invalid_from_utf8::*; use let_underscore::*; @@ -228,6 +230,7 @@ fn lint_mod(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) { MissingDoc: MissingDoc, AsyncFnInTrait: AsyncFnInTrait, NonLocalDefinitions: NonLocalDefinitions::default(), + ImplTraitOvercaptures: ImplTraitOvercaptures, ] ] ); From 554becc180b42c41c35a25811cdc7059c7acc4fb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Apr 2024 21:19:22 -0400 Subject: [PATCH 2/6] Add some commenting --- .../rustc_lint/src/impl_trait_overcaptures.rs | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index a633f355f29..b5fa322d89f 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -24,8 +24,8 @@ } declare_lint_pass!( - /// Lint for use of `async fn` in the definition of a publicly-reachable - /// trait. + /// Lint for opaque types that will begin capturing in-scope but unmentioned lifetimes + /// in edition 2024. ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES] ); @@ -50,7 +50,7 @@ fn check_fn( } } DefKind::Fn => { - // All freee functions need to check for overcaptures. + // All free functions need to check for overcaptures. } DefKind::Closure => return, kind => { @@ -64,6 +64,7 @@ fn check_fn( let sig = cx.tcx.fn_sig(parent_def_id).instantiate_identity(); let mut in_scope_parameters = FxIndexSet::default(); + // Populate the in_scope_parameters list first with all of the generics in scope let mut current_def_id = Some(parent_def_id.to_def_id()); while let Some(def_id) = current_def_id { let generics = cx.tcx.generics_of(def_id); @@ -73,6 +74,8 @@ fn check_fn( current_def_id = generics.parent; } + // Then visit the signature to walk through all the binders (incl. the late-bound + // vars on the function itself, which we need to count too). sig.visit_with(&mut VisitOpaqueTypes { tcx: cx.tcx, parent_def_id, @@ -94,6 +97,7 @@ fn visit_binder>>( &mut self, t: &ty::Binder<'tcx, T>, ) -> Self::Result { + // When we get into a binder, we need to add its own bound vars to the scope. let mut added = vec![]; for arg in t.bound_vars() { let arg: ty::BoundVariableKind = arg; @@ -117,6 +121,8 @@ fn visit_binder>>( t.super_visit_with(self); + // And remove them. The `shift_remove` should be `O(1)` since we're popping + // them off from the end. for arg in added.into_iter().rev() { self.in_scope_parameters.shift_remove(&arg); } @@ -129,22 +135,29 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() && let Some(opaque_def_id) = opaque_ty.def_id.as_local() + // Don't recurse infinitely on an opaque && self.seen.insert(opaque_def_id) + // If it's owned by this function && let opaque = self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty() && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin && parent_def_id == self.parent_def_id + // And if the opaque doesn't already have `use<>` syntax on it... && opaque.precise_capturing_args.is_none() { + // Compute the set of args that are captured by the opaque... let mut captured = UnordSet::default(); let variances = self.tcx.variances_of(opaque_def_id); let mut current_def_id = Some(opaque_def_id.to_def_id()); while let Some(def_id) = current_def_id { let generics = self.tcx.generics_of(def_id); - for param in &generics.params { + for param in &generics.own_params { + // A param is captured if it's invariant. if variances[param.index as usize] != ty::Invariant { continue; } + // We need to turn all `ty::Param`/`ConstKind::Param` and + // `ReEarlyParam`/`ReBound` into def ids. captured.insert(extract_def_id_from_arg( self.tcx, generics, @@ -154,6 +167,8 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { current_def_id = generics.parent; } + // Compute the set of in scope params that are not captured. Get their spans, + // since that's all we really care about them for emitting the diagnostic. let uncaptured_spans: Vec<_> = self .in_scope_parameters .iter() @@ -174,6 +189,10 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { ); } + // Walk into the bounds of the opaque, too, since we want to get nested opaques + // in this lint as well. Interestingly, one place that I expect this lint to fire + // is for `impl for<'a> Bound`, since `impl Other` will begin + // to capture `'a` in e2024 (even though late-bound vars in opaques are not allowed). for clause in self.tcx.item_bounds(opaque_ty.def_id).iter_instantiated(self.tcx, opaque_ty.args) { From 6afe1352d934a15d1cfacf7eaef774898e76011f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Apr 2024 21:57:07 -0400 Subject: [PATCH 3/6] Suggest adding use<> syntax --- compiler/rustc_lint/messages.ftl | 1 + .../rustc_lint/src/impl_trait_overcaptures.rs | 47 ++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 67b2121e5c0..d54e9a3c6eb 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -15,6 +15,7 @@ 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_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future` diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index b5fa322d89f..7272618c5f2 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,5 +1,5 @@ -use rustc_data_structures::{fx::FxIndexSet, unord::UnordSet}; -use rustc_errors::LintDiagnostic; +use rustc_data_structures::fx::FxIndexSet; +use rustc_errors::{Applicability, LintDiagnostic}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -7,7 +7,9 @@ use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; -use rustc_span::Span; +use rustc_session::lint::FutureIncompatibilityReason; +use rustc_span::edition::Edition; +use rustc_span::{BytePos, Span}; use crate::fluent_generated as fluent; use crate::{LateContext, LateLintPass}; @@ -146,7 +148,7 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { && opaque.precise_capturing_args.is_none() { // Compute the set of args that are captured by the opaque... - let mut captured = UnordSet::default(); + let mut captured = FxIndexSet::default(); let variances = self.tcx.variances_of(opaque_def_id); let mut current_def_id = Some(opaque_def_id.to_def_id()); while let Some(def_id) = current_def_id { @@ -172,19 +174,43 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { let uncaptured_spans: Vec<_> = self .in_scope_parameters .iter() - .filter(|def_id| !captured.contains(def_id)) + .filter(|def_id| !captured.contains(*def_id)) .map(|def_id| self.tcx.def_span(def_id)) .collect(); if !uncaptured_spans.is_empty() { + let opaque_span = self.tcx.def_span(opaque_def_id); + + 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(); + Some(( + format!(" use<{}>", generics.join(", ")), + opaque_span.with_lo(opaque_span.lo() + BytePos(4)).shrink_to_lo(), + )) + } else { + None + }; + self.tcx.emit_node_lint( IMPL_TRAIT_OVERCAPTURES, self.tcx.local_def_id_to_hir_id(opaque_def_id), ImplTraitOvercapturesLint { - opaque_span: self.tcx.def_span(opaque_def_id), + opaque_span, self_ty: t, num_captured: uncaptured_spans.len(), uncaptured_spans, + suggestion, }, ); } @@ -209,6 +235,7 @@ struct ImplTraitOvercapturesLint<'tcx> { uncaptured_spans: Vec, self_ty: Ty<'tcx>, num_captured: usize, + suggestion: Option<(String, Span)>, } impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { @@ -218,6 +245,14 @@ fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) { .span(self.opaque_span) .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, + ); + } } fn msg(&self) -> rustc_errors::DiagMessage { From f3fb727b08587e75a6adacca1a8f06f62e31d87e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 18 Apr 2024 22:04:14 -0400 Subject: [PATCH 4/6] Don't suggest using use<> syntax to capture APITs --- compiler/rustc_lint/src/impl_trait_overcaptures.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 7272618c5f2..7659b6da181 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -194,10 +194,15 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { .chain(others) .map(|def_id| self.tcx.item_name(def_id).to_string()) .collect(); - Some(( - format!(" use<{}>", generics.join(", ")), - opaque_span.with_lo(opaque_span.lo() + BytePos(4)).shrink_to_lo(), - )) + // 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.with_lo(opaque_span.lo() + BytePos(4)).shrink_to_lo(), + )) + } else { + None + } } else { None }; From 1529c661e4f1f5294877a471e388d9097e742414 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 20 Apr 2024 11:22:06 -0400 Subject: [PATCH 5/6] Warn against redundant use<...> --- compiler/rustc_ast_lowering/src/lib.rs | 15 +- compiler/rustc_hir/src/hir.rs | 11 +- compiler/rustc_hir/src/intravisit.rs | 2 +- .../rustc_hir_analysis/src/check/check.rs | 2 +- compiler/rustc_lint/messages.ftl | 19 +- .../rustc_lint/src/impl_trait_overcaptures.rs | 215 ++++++++++++------ compiler/rustc_parse/src/parser/ty.rs | 58 ++--- .../impl-trait/precise-capturing/apit.stderr | 2 +- .../impl-trait/precise-capturing/redundant.rs | 25 ++ .../precise-capturing/redundant.stderr | 45 ++++ 10 files changed, 276 insertions(+), 118 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/redundant.rs create mode 100644 tests/ui/impl-trait/precise-capturing/redundant.stderr diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index a76935761f0..c23da8aa01e 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1407,7 +1407,7 @@ fn lower_ty_direct(&mut self, t: &Ty, itctx: ImplTraitContext) -> hir::Ty<'hir> bounds, fn_kind, itctx, - precise_capturing.as_deref().map(|(args, _)| args.as_slice()), + precise_capturing.as_deref().map(|(args, span)| (args.as_slice(), *span)), ), ImplTraitContext::Universal => { if let Some(&(_, span)) = precise_capturing.as_deref() { @@ -1523,7 +1523,7 @@ fn lower_opaque_impl_trait( bounds: &GenericBounds, fn_kind: Option, itctx: ImplTraitContext, - precise_capturing_args: Option<&[PreciseCapturingArg]>, + precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>, ) -> hir::TyKind<'hir> { // Make sure we know that some funky desugaring has been going on here. // This is a first: there is code in other places like for loop @@ -1533,7 +1533,7 @@ fn lower_opaque_impl_trait( let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None); let captured_lifetimes_to_duplicate = - if let Some(precise_capturing) = precise_capturing_args { + if let Some((precise_capturing, _)) = precise_capturing_args { // We'll actually validate these later on; all we need is the list of // lifetimes to duplicate during this portion of lowering. precise_capturing @@ -1607,7 +1607,7 @@ fn lower_opaque_inner( captured_lifetimes_to_duplicate: FxIndexSet, span: Span, opaque_ty_span: Span, - precise_capturing_args: Option<&[PreciseCapturingArg]>, + precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>, lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>], ) -> hir::TyKind<'hir> { let opaque_ty_def_id = self.create_def( @@ -1698,8 +1698,11 @@ fn lower_opaque_inner( this.with_remapping(captured_to_synthesized_mapping, |this| { ( lower_item_bounds(this), - precise_capturing_args.map(|precise_capturing| { - this.lower_precise_capturing_args(precise_capturing) + precise_capturing_args.map(|(precise_capturing, span)| { + ( + this.lower_precise_capturing_args(precise_capturing), + this.lower_span(span), + ) }), ) }); diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 7d991e21ff3..6e4cef068c5 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2631,7 +2631,7 @@ pub struct OpaqueTy<'hir> { /// lowered as an associated type. pub in_trait: bool, /// List of arguments captured via `impl use<'a, P, ...> Trait` syntax. - pub precise_capturing_args: Option<&'hir [PreciseCapturingArg<'hir>]>, + pub precise_capturing_args: Option<(&'hir [PreciseCapturingArg<'hir>], Span)>, } #[derive(Debug, Clone, Copy, HashStable_Generic)] @@ -2641,6 +2641,15 @@ pub enum PreciseCapturingArg<'hir> { Param(PreciseCapturingNonLifetimeArg), } +impl PreciseCapturingArg<'_> { + pub fn hir_id(self) -> HirId { + match self { + PreciseCapturingArg::Lifetime(lt) => lt.hir_id, + PreciseCapturingArg::Param(param) => param.hir_id, + } + } +} + /// We need to have a [`Node`] for the [`HirId`] that we attach the type/const param /// resolution to. Lifetimes don't have this problem, and for them, it's actually /// kind of detrimental to use a custom node type versus just using [`Lifetime`], diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 0b095db953b..664784cd2c6 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -533,7 +533,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: try_visit!(visitor.visit_id(item.hir_id())); try_visit!(walk_generics(visitor, generics)); walk_list!(visitor, visit_param_bound, bounds); - if let Some(precise_capturing_args) = precise_capturing_args { + if let Some((precise_capturing_args, _)) = precise_capturing_args { for arg in precise_capturing_args { try_visit!(visitor.visit_precise_capturing_arg(arg)); } diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 652c1885073..b5c06751405 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -486,7 +486,7 @@ fn sanity_check_found_hidden_type<'tcx>( fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDefId) { let hir::OpaqueTy { precise_capturing_args, .. } = *tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty(); - let Some(precise_capturing_args) = precise_capturing_args else { + let Some((precise_capturing_args, _)) = precise_capturing_args else { // No precise capturing args; nothing to validate return; }; diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index d54e9a3c6eb..5180fce2eb3 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -9,14 +9,6 @@ lint_array_into_iter = .use_explicit_into_iter_suggestion = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value -lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024 - .note = specifically, {$num_captured -> - [one] this lifetime is - *[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_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future` .suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change @@ -277,6 +269,17 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level +lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024 + .note = specifically, {$num_captured -> + [one] this lifetime is + *[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 + lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe .label = not FFI-safe .note = the type is defined here diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 7659b6da181..a239f38006b 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,9 +1,11 @@ use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::unord::UnordSet; use rustc_errors::{Applicability, LintDiagnostic}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::intravisit; +use rustc_macros::LintDiagnostic; +use rustc_middle::middle::resolve_bound_vars::ResolvedArg; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; @@ -14,10 +16,12 @@ use crate::fluent_generated as fluent; use crate::{LateContext, LateLintPass}; +// TODO: feature gate these too + declare_lint! { /// UwU pub IMPL_TRAIT_OVERCAPTURES, - Warn, + Allow, "will capture more lifetimes than possibly intended in edition 2024", @future_incompatible = FutureIncompatibleInfo { reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), @@ -25,66 +29,64 @@ }; } +declare_lint! { + /// UwU + pub IMPL_TRAIT_REDUNDANT_CAPTURES, + Warn, + "uwu 2" +} + declare_lint_pass!( /// Lint for opaque types that will begin capturing in-scope but unmentioned lifetimes /// in edition 2024. - ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES] + ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES, IMPL_TRAIT_REDUNDANT_CAPTURES] ); impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - _: intravisit::FnKind<'tcx>, - _: &'tcx hir::FnDecl<'tcx>, - _: &'tcx hir::Body<'tcx>, - _: Span, - parent_def_id: LocalDefId, - ) { - match cx.tcx.def_kind(parent_def_id) { - DefKind::AssocFn => { - // RPITITs already capture all lifetimes in scope, so skip them. - if matches!( - cx.tcx.def_kind(cx.tcx.local_parent(parent_def_id)), - DefKind::Trait | DefKind::Impl { of_trait: true } - ) { - return; - } - } - DefKind::Fn => { - // All free functions need to check for overcaptures. - } - DefKind::Closure => return, - kind => { - unreachable!( - "expected function item, found {}", - kind.descr(parent_def_id.to_def_id()) - ) - } + fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'tcx>) { + match &it.kind { + hir::ItemKind::Fn(..) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} } - - let sig = cx.tcx.fn_sig(parent_def_id).instantiate_identity(); - - let mut in_scope_parameters = FxIndexSet::default(); - // Populate the in_scope_parameters list first with all of the generics in scope - let mut current_def_id = Some(parent_def_id.to_def_id()); - while let Some(def_id) = current_def_id { - let generics = cx.tcx.generics_of(def_id); - for param in &generics.params { - in_scope_parameters.insert(param.def_id); - } - current_def_id = generics.parent; - } - - // Then visit the signature to walk through all the binders (incl. the late-bound - // vars on the function itself, which we need to count too). - sig.visit_with(&mut VisitOpaqueTypes { - tcx: cx.tcx, - parent_def_id, - in_scope_parameters, - seen: Default::default(), - }); } + + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::ImplItem<'tcx>) { + match &it.kind { + hir::ImplItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} + } + } + + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::TraitItem<'tcx>) { + match &it.kind { + hir::TraitItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} + } + } +} + +fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { + let sig = tcx.fn_sig(parent_def_id).instantiate_identity(); + + let mut in_scope_parameters = FxIndexSet::default(); + // Populate the in_scope_parameters list first with all of the generics in scope + let mut current_def_id = Some(parent_def_id.to_def_id()); + while let Some(def_id) = current_def_id { + let generics = tcx.generics_of(def_id); + for param in &generics.own_params { + in_scope_parameters.insert(param.def_id); + } + current_def_id = generics.parent; + } + + // Then visit the signature to walk through all the binders (incl. the late-bound + // vars on the function itself, which we need to count too). + sig.visit_with(&mut VisitOpaqueTypes { + tcx, + parent_def_id, + in_scope_parameters, + seen: Default::default(), + }); } struct VisitOpaqueTypes<'tcx> { @@ -109,14 +111,11 @@ fn visit_binder>>( let unique = self.in_scope_parameters.insert(def_id); assert!(unique); } - ty::BoundVariableKind::Ty(_) => { - todo!("we don't support late-bound type params in `impl Trait`") - } - ty::BoundVariableKind::Region(..) => { - unreachable!("all AST-derived bound regions should have a name") - } - ty::BoundVariableKind::Const => { - unreachable!("non-lifetime binder consts are not allowed") + _ => { + self.tcx.dcx().span_delayed_bug( + self.tcx.def_span(self.parent_def_id), + format!("unsupported bound variable kind: {arg:?}"), + ); } } } @@ -131,11 +130,19 @@ fn visit_binder>>( } fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { - if !t.has_opaque_types() { + if !t.has_aliases() { return; } - if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() + if let ty::Alias(ty::Projection, opaque_ty) = *t.kind() + && self.tcx.is_impl_trait_in_trait(opaque_ty.def_id) + { + // visit the opaque of the RPITIT + self.tcx + .type_of(opaque_ty.def_id) + .instantiate(self.tcx, opaque_ty.args) + .visit_with(self) + } else if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() && let Some(opaque_def_id) = opaque_ty.def_id.as_local() // Don't recurse infinitely on an opaque && self.seen.insert(opaque_def_id) @@ -144,8 +151,6 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty() && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin && parent_def_id == self.parent_def_id - // And if the opaque doesn't already have `use<>` syntax on it... - && opaque.precise_capturing_args.is_none() { // Compute the set of args that are captured by the opaque... let mut captured = FxIndexSet::default(); @@ -178,9 +183,16 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { .map(|def_id| self.tcx.def_span(def_id)) .collect(); - if !uncaptured_spans.is_empty() { - let opaque_span = self.tcx.def_span(opaque_def_id); + let opaque_span = self.tcx.def_span(opaque_def_id); + let new_capture_rules = + opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024; + // 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 !new_capture_rules + && opaque.precise_capturing_args.is_none() + && !uncaptured_spans.is_empty() + { let suggestion = if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(opaque_span) && snippet.starts_with("impl ") @@ -207,11 +219,11 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { None }; - self.tcx.emit_node_lint( + self.tcx.emit_node_span_lint( IMPL_TRAIT_OVERCAPTURES, self.tcx.local_def_id_to_hir_id(opaque_def_id), + opaque_span, ImplTraitOvercapturesLint { - opaque_span, self_ty: t, num_captured: uncaptured_spans.len(), uncaptured_spans, @@ -219,6 +231,60 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { }, ); } + // Otherwise, if we are edition 2024, have `use<>` syntax, and + // have no uncaptured args, then we should warn to the user that + // it's redundant to capture all args explicitly. + else if new_capture_rules + && let Some((captured_args, capturing_span)) = opaque.precise_capturing_args + { + let mut explicitly_captured = UnordSet::default(); + for arg in captured_args { + match self.tcx.named_bound_var(arg.hir_id()) { + Some( + ResolvedArg::EarlyBound(def_id) | ResolvedArg::LateBound(_, _, def_id), + ) => { + if self.tcx.def_kind(self.tcx.parent(def_id)) == DefKind::OpaqueTy { + let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) + | ty::ReLateParam(ty::LateParamRegion { + bound_region: ty::BoundRegionKind::BrNamed(def_id, _), + .. + })) = self + .tcx + .map_opaque_lifetime_to_parent_lifetime(def_id.expect_local()) + .kind() + else { + span_bug!( + self.tcx.def_span(def_id), + "variable should have been duplicated from a parent" + ); + }; + explicitly_captured.insert(def_id); + } else { + explicitly_captured.insert(def_id); + } + } + _ => { + self.tcx.dcx().span_delayed_bug( + self.tcx().hir().span(arg.hir_id()), + "no valid for captured arg", + ); + } + } + } + + if self + .in_scope_parameters + .iter() + .all(|def_id| explicitly_captured.contains(def_id)) + { + self.tcx.emit_node_span_lint( + IMPL_TRAIT_REDUNDANT_CAPTURES, + self.tcx.local_def_id_to_hir_id(opaque_def_id), + opaque_span, + ImplTraitRedundantCapturesLint { capturing_span }, + ); + } + } // Walk into the bounds of the opaque, too, since we want to get nested opaques // in this lint as well. Interestingly, one place that I expect this lint to fire @@ -236,7 +302,6 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { } struct ImplTraitOvercapturesLint<'tcx> { - opaque_span: Span, uncaptured_spans: Vec, self_ty: Ty<'tcx>, num_captured: usize, @@ -247,7 +312,6 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) { diag.arg("self_ty", self.self_ty.to_string()) .arg("num_captured", self.num_captured) - .span(self.opaque_span) .span_note(self.uncaptured_spans, fluent::lint_note) .note(fluent::lint_note2); if let Some((suggestion, span)) = self.suggestion { @@ -265,6 +329,13 @@ fn msg(&self) -> rustc_errors::DiagMessage { } } +#[derive(LintDiagnostic)] +#[diag(lint_impl_trait_redundant_captures)] +struct ImplTraitRedundantCapturesLint { + #[suggestion(lint_suggestion, code = "", applicability = "machine-applicable")] + capturing_span: Span, +} + fn extract_def_id_from_arg<'tcx>( tcx: TyCtxt<'tcx>, generics: &'tcx ty::Generics, diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 7096b201f84..2f08a48c7bc 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -675,8 +675,8 @@ fn parse_impl_ty(&mut self, impl_dyn_multi: &mut bool) -> PResult<'a, TyKind> { let precise_capturing = if self.eat_keyword(kw::Use) { let use_span = self.prev_token.span; self.psess.gated_spans.gate(sym::precise_capturing, use_span); - let args = self.parse_precise_capturing_args()?; - Some(P((args, use_span))) + let (args, args_span) = self.parse_precise_capturing_args()?; + Some(P((args, use_span.to(args_span)))) } else { None }; @@ -689,32 +689,34 @@ fn parse_impl_ty(&mut self, impl_dyn_multi: &mut bool) -> PResult<'a, TyKind> { Ok(TyKind::ImplTrait(ast::DUMMY_NODE_ID, bounds, precise_capturing)) } - fn parse_precise_capturing_args(&mut self) -> PResult<'a, ThinVec> { - Ok(self - .parse_unspanned_seq( - &TokenKind::Lt, - &TokenKind::Gt, - SeqSep::trailing_allowed(token::Comma), - |self_| { - if self_.check_keyword(kw::SelfUpper) { - self_.bump(); - Ok(PreciseCapturingArg::Arg( - ast::Path::from_ident(self_.prev_token.ident().unwrap().0), - DUMMY_NODE_ID, - )) - } else if self_.check_ident() { - Ok(PreciseCapturingArg::Arg( - ast::Path::from_ident(self_.parse_ident()?), - DUMMY_NODE_ID, - )) - } else if self_.check_lifetime() { - Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime())) - } else { - self_.unexpected_any() - } - }, - )? - .0) + fn parse_precise_capturing_args( + &mut self, + ) -> PResult<'a, (ThinVec, Span)> { + let lo = self.token.span; + let (args, _) = self.parse_unspanned_seq( + &TokenKind::Lt, + &TokenKind::Gt, + SeqSep::trailing_allowed(token::Comma), + |self_| { + if self_.check_keyword(kw::SelfUpper) { + self_.bump(); + Ok(PreciseCapturingArg::Arg( + ast::Path::from_ident(self_.prev_token.ident().unwrap().0), + DUMMY_NODE_ID, + )) + } else if self_.check_ident() { + Ok(PreciseCapturingArg::Arg( + ast::Path::from_ident(self_.parse_ident()?), + DUMMY_NODE_ID, + )) + } else if self_.check_lifetime() { + Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime())) + } else { + self_.unexpected_any() + } + }, + )?; + Ok((args, lo.to(self.prev_token.span))) } /// Is a `dyn B0 + ... + Bn` type allowed here? diff --git a/tests/ui/impl-trait/precise-capturing/apit.stderr b/tests/ui/impl-trait/precise-capturing/apit.stderr index 36bf80d9e2f..96548f5732f 100644 --- a/tests/ui/impl-trait/precise-capturing/apit.stderr +++ b/tests/ui/impl-trait/precise-capturing/apit.stderr @@ -11,7 +11,7 @@ error: `use<...>` precise capturing syntax not allowed on argument-position `imp --> $DIR/apit.rs:4:18 | LL | fn hello(_: impl use<> Sized) {} - | ^^^ + | ^^^^^ error: aborting due to 1 previous error; 1 warning emitted diff --git a/tests/ui/impl-trait/precise-capturing/redundant.rs b/tests/ui/impl-trait/precise-capturing/redundant.rs new file mode 100644 index 00000000000..108a4cb64aa --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/redundant.rs @@ -0,0 +1,25 @@ +//@ compile-flags: -Zunstable-options --edition=2024 +//@ check-pass + +#![feature(precise_capturing)] +//~^ WARN the feature `precise_capturing` is incomplete + +fn hello<'a>() -> impl use<'a> Sized {} +//~^ WARN all possible in-scope parameters are already captured + +struct Inherent; +impl Inherent { + fn inherent(&self) -> impl use<'_> Sized {} + //~^ WARN all possible in-scope parameters are already captured +} + +trait Test<'a> { + fn in_trait() -> impl use<'a, Self> Sized; + //~^ WARN all possible in-scope parameters are already captured +} +impl<'a> Test<'a> for () { + fn in_trait() -> impl use<'a> Sized {} + //~^ WARN all possible in-scope parameters are already captured +} + +fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/redundant.stderr b/tests/ui/impl-trait/precise-capturing/redundant.stderr new file mode 100644 index 00000000000..325f04d3536 --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/redundant.stderr @@ -0,0 +1,45 @@ +warning: the feature `precise_capturing` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/redundant.rs:4:12 + | +LL | #![feature(precise_capturing)] + | ^^^^^^^^^^^^^^^^^ + | + = note: see issue #123432 for more information + = note: `#[warn(incomplete_features)]` on by default + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:7:19 + | +LL | fn hello<'a>() -> impl use<'a> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + | + = note: `#[warn(impl_trait_redundant_captures)]` on by default + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:12:27 + | +LL | fn inherent(&self) -> impl use<'_> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:17:22 + | +LL | fn in_trait() -> impl use<'a, Self> Sized; + | ^^^^^-------------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:21:22 + | +LL | fn in_trait() -> impl use<'a> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: 5 warnings emitted + From 052de1da4f579f63d2403f8045f2d6e342a90200 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 21 Apr 2024 11:27:27 -0400 Subject: [PATCH 6/6] And finally add tests --- .../rustc_lint/src/impl_trait_overcaptures.rs | 89 +++++++++++++++---- .../precise-capturing/overcaptures-2024.fixed | 29 ++++++ .../precise-capturing/overcaptures-2024.rs | 29 ++++++ .../overcaptures-2024.stderr | 75 ++++++++++++++++ 4 files changed, 207 insertions(+), 15 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed create mode 100644 tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs create mode 100644 tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index a239f38006b..30bf80b915b 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -9,31 +9,89 @@ use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; -use rustc_session::lint::FutureIncompatibilityReason; -use rustc_span::edition::Edition; -use rustc_span::{BytePos, Span}; +use rustc_middle::{bug, span_bug}; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::{sym, BytePos, Span}; use crate::fluent_generated as fluent; use crate::{LateContext, LateLintPass}; -// TODO: feature gate these too - declare_lint! { - /// UwU + /// The `impl_trait_overcaptures` lint warns against cases where lifetime + /// capture behavior will differ in edition 2024. + /// + /// In the 2024 edition, `impl Trait`s will capture all lifetimes in scope, + /// rather than just the lifetimes that are mentioned in the bounds of the type. + /// Often these sets are equal, but if not, it means that the `impl Trait` may + /// cause erroneous borrow-checker errors. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// # #![feature(precise_capturing)] + /// # #![allow(incomplete_features)] + /// # #![deny(impl_trait_overcaptures)] + /// # use std::fmt::Display; + /// let mut x = vec![]; + /// x.push(1); + /// + /// fn test(x: &Vec) -> impl Display { + /// x[0] + /// } + /// + /// let element = test(&x); + /// x.push(2); + /// println!("{element}"); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// In edition < 2024, the returned `impl Display` doesn't capture the + /// lifetime from the `&Vec`, so the vector can be mutably borrowed + /// while the `impl Display` is live. + /// + /// To fix this, we can explicitly state that the `impl Display` doesn't + /// capture any lifetimes, using `impl use<> Display`. pub IMPL_TRAIT_OVERCAPTURES, Allow, - "will capture more lifetimes than possibly intended in edition 2024", - @future_incompatible = FutureIncompatibleInfo { - reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), - reference: "", - }; + "`impl Trait` will capture more lifetimes than possibly intended in edition 2024", + @feature_gate = sym::precise_capturing; + //@future_incompatible = FutureIncompatibleInfo { + // reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), + // reference: "", + //}; } declare_lint! { - /// UwU + /// The `impl_trait_redundant_captures` lint warns against cases where use of the + /// precise capturing `use<...>` syntax is not needed. + /// + /// In the 2024 edition, `impl Trait`s will capture all lifetimes in scope. + /// If precise-capturing `use<...>` syntax is used, and the set of parameters + /// that are captures are *equal* to the set of parameters in scope, then + /// the syntax is redundant, and can be removed. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// # #![feature(precise_capturing, lifetime_capture_rules_2024)] + /// # #![allow(incomplete_features)] + /// # #![deny(impl_trait_redundant_captures)] + /// fn test<'a>(x: &'a i32) -> impl use<'a> Sized { x } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// To fix this, remove the `use<'a>`, since the lifetime is already captured + /// since it is in scope. pub IMPL_TRAIT_REDUNDANT_CAPTURES, Warn, - "uwu 2" + "redundant precise-capturing `use<...>` syntax on an `impl Trait`", + @feature_gate = sym::precise_capturing; } declare_lint_pass!( @@ -106,7 +164,8 @@ fn visit_binder>>( for arg in t.bound_vars() { let arg: ty::BoundVariableKind = arg; match arg { - ty::BoundVariableKind::Region(ty::BoundRegionKind::BrNamed(def_id, ..)) => { + ty::BoundVariableKind::Region(ty::BoundRegionKind::BrNamed(def_id, ..)) + | ty::BoundVariableKind::Ty(ty::BoundTyKind::Param(def_id, _)) => { added.push(def_id); let unique = self.in_scope_parameters.insert(def_id); assert!(unique); @@ -265,7 +324,7 @@ fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { } _ => { self.tcx.dcx().span_delayed_bug( - self.tcx().hir().span(arg.hir_id()), + self.tcx.hir().span(arg.hir_id()), "no valid for captured arg", ); } diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed new file mode 100644 index 00000000000..014ab23e4eb --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.fixed @@ -0,0 +1,29 @@ +//@ run-rustfix + +#![feature(precise_capturing)] +#![allow(unused, incomplete_features)] +#![deny(impl_trait_overcaptures)] + +fn named<'a>(x: &'a i32) -> impl use<> Sized { *x } +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + +fn implicit(x: &i32) -> impl use<> Sized { *x } +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + +struct W; +impl W { + fn hello(&self, x: &i32) -> impl use<'_> Sized + '_ { self } + //~^ ERROR `impl Sized + '_` will capture more lifetimes than possibly intended in edition 2024 +} + +trait Higher<'a> { + type Output; +} +impl Higher<'_> for () { + type Output = (); +} + +fn hrtb() -> impl for<'a> Higher<'a, Output = impl use<> Sized> {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 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 new file mode 100644 index 00000000000..e4b7828d60f --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.rs @@ -0,0 +1,29 @@ +//@ run-rustfix + +#![feature(precise_capturing)] +#![allow(unused, incomplete_features)] +#![deny(impl_trait_overcaptures)] + +fn named<'a>(x: &'a i32) -> impl Sized { *x } +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + +fn implicit(x: &i32) -> impl Sized { *x } +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + +struct W; +impl W { + fn hello(&self, x: &i32) -> impl Sized + '_ { self } + //~^ ERROR `impl Sized + '_` will capture more lifetimes than possibly intended in edition 2024 +} + +trait Higher<'a> { + type Output; +} +impl Higher<'_> for () { + type Output = (); +} + +fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} +//~^ ERROR `impl Sized` will capture more lifetimes than possibly intended in edition 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 new file mode 100644 index 00000000000..16cb8b7e94b --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024.stderr @@ -0,0 +1,75 @@ +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:7:29 + | +LL | fn named<'a>(x: &'a i32) -> impl Sized { *x } + | ^^^^^^^^^^ + | +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:7:10 + | +LL | fn named<'a>(x: &'a i32) -> impl Sized { *x } + | ^^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +note: the lint level is defined here + --> $DIR/overcaptures-2024.rs:5:9 + | +LL | #![deny(impl_trait_overcaptures)] + | ^^^^^^^^^^^^^^^^^^^^^^^ +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn named<'a>(x: &'a i32) -> impl use<> Sized { *x } + | +++++ + +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:10:25 + | +LL | fn implicit(x: &i32) -> impl Sized { *x } + | ^^^^^^^^^^ + | +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:10:16 + | +LL | fn implicit(x: &i32) -> impl Sized { *x } + | ^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn implicit(x: &i32) -> impl use<> Sized { *x } + | +++++ + +error: `impl Sized + '_` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:15:33 + | +LL | fn hello(&self, x: &i32) -> impl Sized + '_ { self } + | ^^^^^^^^^^^^^^^ + | +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:15:24 + | +LL | fn hello(&self, x: &i32) -> impl Sized + '_ { self } + | ^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn hello(&self, x: &i32) -> impl use<'_> Sized + '_ { self } + | +++++++ + +error: `impl Sized` will capture more lifetimes than possibly intended in edition 2024 + --> $DIR/overcaptures-2024.rs:26:47 + | +LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} + | ^^^^^^^^^^ + | +note: specifically, this lifetime is in scope but not mentioned in the type's bounds + --> $DIR/overcaptures-2024.rs:26:23 + | +LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl Sized> {} + | ^^ + = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024 +help: use the precise capturing `use<...>` syntax to make the captures explicit + | +LL | fn hrtb() -> impl for<'a> Higher<'a, Output = impl use<> Sized> {} + | +++++ + +error: aborting due to 4 previous errors +