From c1d041036e73ba514ca5fa1a8d2778ded416d3aa Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 19 Aug 2024 13:17:59 -0400 Subject: [PATCH] Review comments --- .../rustc_lint/src/impl_trait_overcaptures.rs | 87 +++++++++---------- compiler/rustc_lint/src/lib.rs | 1 + 2 files changed, 40 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 98330792673..c43c650a9f9 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,3 +1,4 @@ +use std::assert_matches::debug_assert_matches; use std::cell::LazyCell; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; @@ -178,7 +179,7 @@ fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { ambient_variance: ty::Covariant, generics: tcx.generics_of(parent_def_id), }; - let _ = functional_variances.relate(sig, sig); + functional_variances.relate(sig, sig).unwrap(); functional_variances.variances }), outlives_env: LazyCell::new(|| { @@ -208,10 +209,7 @@ impl<'tcx, VarFn, OutlivesFn> TypeVisitor> VarFn: FnOnce() -> FxHashMap, OutlivesFn: FnOnce() -> OutlivesEnvironment<'tcx>, { - fn visit_binder>>( - &mut self, - t: &ty::Binder<'tcx, T>, - ) -> Self::Result { + fn visit_binder>>(&mut self, t: &ty::Binder<'tcx, T>) { // 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() { @@ -241,7 +239,7 @@ fn visit_binder>>( } } - fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { + fn visit_ty(&mut self, t: Ty<'tcx>) { if !t.has_aliases() { return; } @@ -293,50 +291,43 @@ 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. + // Compute the set of in scope params that are not captured. let mut uncaptured_args: FxIndexSet<_> = self .in_scope_parameters .iter() .filter(|&(def_id, _)| !captured.contains(def_id)) .collect(); - - // These are args that we know are likely fine to "overcapture", since they can be - // contravariantly shortened to one of the already-captured lifetimes that they - // outlive. - let covariant_long_args: FxIndexSet<_> = uncaptured_args - .iter() - .copied() - .filter(|&(def_id, kind)| { - let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) - else { - return false; - }; - let DefKind::LifetimeParam = self.tcx.def_kind(def_id) else { - return false; - }; - let uncaptured = match *kind { - ParamKind::Early(name, index) => ty::Region::new_early_param( - self.tcx, - ty::EarlyParamRegion { name, index }, - ), - ParamKind::Free(def_id, name) => ty::Region::new_late_param( - self.tcx, - self.parent_def_id.to_def_id(), - ty::BoundRegionKind::BrNamed(def_id, name), - ), - ParamKind::Late => return false, - }; - // Does this region outlive any captured region? - captured_regions.iter().any(|r| { - self.outlives_env - .free_region_map() - .sub_free_regions(self.tcx, *r, uncaptured) - }) + // Remove the set of lifetimes that are in-scope that outlive some other captured + // lifetime and are contravariant (i.e. covariant in argument position). + uncaptured_args.retain(|&(def_id, kind)| { + let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) else { + // Keep all covariant/invariant args. Also if variance is `None`, + // then that means it's either not a lifetime, or it didn't show up + // anywhere in the signature. + return true; + }; + // We only computed variance of lifetimes... + debug_assert_matches!(self.tcx.def_kind(def_id), DefKind::LifetimeParam); + let uncaptured = match *kind { + ParamKind::Early(name, index) => ty::Region::new_early_param( + self.tcx, + ty::EarlyParamRegion { name, index }, + ), + ParamKind::Free(def_id, name) => ty::Region::new_late_param( + self.tcx, + self.parent_def_id.to_def_id(), + ty::BoundRegionKind::BrNamed(def_id, name), + ), + // Totally ignore late bound args from binders. + ParamKind::Late => return true, + }; + // Does this region outlive any captured region? + !captured_regions.iter().any(|r| { + self.outlives_env + .free_region_map() + .sub_free_regions(self.tcx, *r, uncaptured) }) - .collect(); - // We don't care to warn on these args. - uncaptured_args.retain(|arg| !covariant_long_args.contains(arg)); + }); // 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. @@ -546,13 +537,13 @@ fn relate_with_variance>>( ) -> RelateResult<'tcx, T> { let old_variance = self.ambient_variance; self.ambient_variance = self.ambient_variance.xform(variance); - self.relate(a, b)?; + self.relate(a, b).unwrap(); self.ambient_variance = old_variance; Ok(a) } fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { - structurally_relate_tys(self, a, b)?; + structurally_relate_tys(self, a, b).unwrap(); Ok(a) } @@ -590,7 +581,7 @@ fn consts( a: ty::Const<'tcx>, b: ty::Const<'tcx>, ) -> RelateResult<'tcx, ty::Const<'tcx>> { - structurally_relate_consts(self, a, b)?; + structurally_relate_consts(self, a, b).unwrap(); Ok(a) } @@ -602,7 +593,7 @@ fn binders( where T: Relate>, { - self.relate(a.skip_binder(), b.skip_binder())?; + self.relate(a.skip_binder(), b.skip_binder()).unwrap(); Ok(a) } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c5a5c5b30af..bb7de4739fb 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -30,6 +30,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] #![feature(array_windows)] +#![feature(assert_matches)] #![feature(box_patterns)] #![feature(control_flow_enum)] #![feature(extract_if)]