Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Sep 5, 2024
1 parent 39998be commit f29b818
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 48 deletions.
87 changes: 39 additions & 48 deletions compiler/rustc_lint/src/impl_trait_overcaptures.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::assert_matches::debug_assert_matches;
use std::cell::LazyCell;

use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -208,10 +209,7 @@ where
VarFn: FnOnce() -> FxHashMap<DefId, ty::Variance>,
OutlivesFn: FnOnce() -> OutlivesEnvironment<'tcx>,
{
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
&mut self,
t: &ty::Binder<'tcx, T>,
) -> Self::Result {
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(&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() {
Expand Down Expand Up @@ -241,7 +239,7 @@ where
}
}

fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
fn visit_ty(&mut self, t: Ty<'tcx>) {
if !t.has_aliases() {
return;
}
Expand Down Expand Up @@ -293,50 +291,43 @@ where
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.
Expand Down Expand Up @@ -546,13 +537,13 @@ impl<'tcx> TypeRelation<TyCtxt<'tcx>> for FunctionalVariances<'tcx> {
) -> 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)
}

Expand Down Expand Up @@ -590,7 +581,7 @@ impl<'tcx> TypeRelation<TyCtxt<'tcx>> for FunctionalVariances<'tcx> {
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)
}

Expand All @@ -602,7 +593,7 @@ impl<'tcx> TypeRelation<TyCtxt<'tcx>> for FunctionalVariances<'tcx> {
where
T: Relate<TyCtxt<'tcx>>,
{
self.relate(a.skip_binder(), b.skip_binder())?;
self.relate(a.skip_binder(), b.skip_binder()).unwrap();
Ok(a)
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit f29b818

Please sign in to comment.