Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of OnlySelfBounds #131939

Merged
merged 1 commit into from
Oct 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions compiler/rustc_hir_analysis/src/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt, Upcast};
use rustc_span::Span;
use rustc_span::def_id::DefId;

use crate::hir_ty_lowering::OnlySelfBounds;
use crate::hir_ty_lowering::PredicateFilter;

/// Collects together a list of type bounds. These lists of bounds occur in many places
/// in Rust's syntax:
Expand Down Expand Up @@ -52,7 +52,7 @@ impl<'tcx> Bounds<'tcx> {
span: Span,
polarity: ty::PredicatePolarity,
constness: ty::BoundConstness,
only_self_bounds: OnlySelfBounds,
predicate_filter: PredicateFilter,
) {
let clause = (
bound_trait_ref
Expand All @@ -72,9 +72,18 @@ impl<'tcx> Bounds<'tcx> {
// FIXME(effects): Lift this out of `push_trait_bound`, and move it somewhere else.
// Perhaps moving this into `lower_poly_trait_ref`, just like we lower associated
// type bounds.
if !tcx.features().effects || only_self_bounds.0 {
if !tcx.features().effects {
return;
}
match predicate_filter {
PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => {
return;
}
PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => {
// Ok.
}
}

// For `T: ~const Tr` or `T: const Tr`, we need to add an additional bound on the
// associated type of `<T as Tr>` and make sure that the effect is compatible.
let compat_val = match (tcx.def_kind(defining_def_id), constness) {
Expand Down
40 changes: 4 additions & 36 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Copy link
Member

@fmease fmease Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete fn bound_defines_assoc_item (now unused, should fail CI, right?) or use it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I inlined it into its one callsite, so I can just delete it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ugh, it wouldn't fail CI because #[instrument] fucks with reachability lol

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh gawd! :(

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::bounds::Bounds;
use crate::collect::ItemCtxt;
use crate::constrained_generic_params as cgp;
use crate::delegation::inherit_predicates_for_delegation_item;
use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter, RegionInferReason};
use crate::hir_ty_lowering::{HirTyLowerer, PredicateFilter, RegionInferReason};

/// Returns a list of all type predicates (explicit and implicit) for the definition with
/// ID `def_id`. This includes all predicates returned by `explicit_predicates_of`, plus
Expand Down Expand Up @@ -270,7 +270,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
bound_pred.bounds.iter(),
&mut bounds,
bound_vars,
OnlySelfBounds(false),
PredicateFilter::All,
);
predicates.extend(bounds.clauses(tcx));
effects_min_tys.extend(bounds.effects_min_tys());
Expand Down Expand Up @@ -825,20 +825,6 @@ impl<'tcx> ItemCtxt<'tcx> {
continue;
};

// Subtle: If we're collecting `SelfAndAssociatedTypeBounds`, then we
// want to only consider predicates with `Self: ...`, but we don't want
// `OnlySelfBounds(true)` since we want to collect the nested associated
// type bound as well.
let (only_self_bounds, assoc_name) = match filter {
PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => {
(OnlySelfBounds(false), None)
}
PredicateFilter::SelfOnly => (OnlySelfBounds(true), None),
PredicateFilter::SelfThatDefines(assoc_name) => {
(OnlySelfBounds(true), Some(assoc_name))
}
};

let bound_ty = if predicate.is_param_bound(param_def_id.to_def_id()) {
ty
} else if matches!(filter, PredicateFilter::All) {
Expand All @@ -850,31 +836,13 @@ impl<'tcx> ItemCtxt<'tcx> {
let bound_vars = self.tcx.late_bound_vars(predicate.hir_id);
self.lowerer().lower_poly_bounds(
bound_ty,
predicate.bounds.iter().filter(|bound| {
assoc_name
.map_or(true, |assoc_name| self.bound_defines_assoc_item(bound, assoc_name))
}),
predicate.bounds.iter(),
&mut bounds,
bound_vars,
only_self_bounds,
filter,
);
}

bounds.clauses(self.tcx).collect()
}

#[instrument(level = "trace", skip(self))]
fn bound_defines_assoc_item(&self, b: &hir::GenericBound<'_>, assoc_name: Ident) -> bool {
match b {
hir::GenericBound::Trait(poly_trait_ref) => {
let trait_ref = &poly_trait_ref.trait_ref;
if let Some(trait_did) = trait_ref.trait_def_id() {
self.tcx.trait_may_define_assoc_item(trait_did, assoc_name)
} else {
false
}
}
_ => false,
}
}
}
83 changes: 38 additions & 45 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ use tracing::{debug, instrument};
use super::errors::GenericsArgsErrExtend;
use crate::bounds::Bounds;
use crate::errors;
use crate::hir_ty_lowering::{
AssocItemQSelf, HirTyLowerer, OnlySelfBounds, PredicateFilter, RegionInferReason,
};
use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer, PredicateFilter, RegionInferReason};

impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Add a `Sized` bound to the `bounds` if appropriate.
Expand Down Expand Up @@ -150,11 +148,25 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
hir_bounds: I,
bounds: &mut Bounds<'tcx>,
bound_vars: &'tcx ty::List<ty::BoundVariableKind>,
only_self_bounds: OnlySelfBounds,
predicate_filter: PredicateFilter,
) where
'tcx: 'hir,
{
for hir_bound in hir_bounds {
// In order to avoid cycles, when we're lowering `SelfThatDefines`,
// we skip over any traits that don't define the given associated type.

if let PredicateFilter::SelfThatDefines(assoc_name) = predicate_filter {
if let Some(trait_ref) = hir_bound.trait_ref()
&& let Some(trait_did) = trait_ref.trait_def_id()
&& self.tcx().trait_may_define_assoc_item(trait_did, assoc_name)
{
// Okay
} else {
continue;
}
}

match hir_bound {
hir::GenericBound::Trait(poly_trait_ref) => {
let (constness, polarity) = match poly_trait_ref.modifiers {
Expand All @@ -179,7 +191,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
polarity,
param_ty,
bounds,
only_self_bounds,
predicate_filter,
);
}
hir::GenericBound::Outlives(lifetime) => {
Expand Down Expand Up @@ -213,37 +225,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
&self,
param_ty: Ty<'tcx>,
hir_bounds: &[hir::GenericBound<'tcx>],
filter: PredicateFilter,
predicate_filter: PredicateFilter,
) -> Bounds<'tcx> {
let mut bounds = Bounds::default();

let only_self_bounds = match filter {
PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => {
OnlySelfBounds(false)
}
PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => OnlySelfBounds(true),
};

self.lower_poly_bounds(
param_ty,
hir_bounds.iter().filter(|bound| match filter {
PredicateFilter::All
| PredicateFilter::SelfOnly
| PredicateFilter::SelfAndAssociatedTypeBounds => true,
PredicateFilter::SelfThatDefines(assoc_name) => {
if let Some(trait_ref) = bound.trait_ref()
&& let Some(trait_did) = trait_ref.trait_def_id()
&& self.tcx().trait_may_define_assoc_item(trait_did, assoc_name)
{
true
} else {
false
}
}
}),
hir_bounds.iter(),
&mut bounds,
ty::List::empty(),
only_self_bounds,
predicate_filter,
);
debug!(?bounds);

Expand All @@ -267,7 +258,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
bounds: &mut Bounds<'tcx>,
duplicates: &mut FxIndexMap<DefId, Span>,
path_span: Span,
only_self_bounds: OnlySelfBounds,
predicate_filter: PredicateFilter,
) -> Result<(), ErrorGuaranteed> {
let tcx = self.tcx();

Expand Down Expand Up @@ -444,21 +435,23 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// Lower a constraint like `Item: Debug` as found in HIR bound `T: Iterator<Item: Debug>`
// to a bound involving a projection: `<T as Iterator>::Item: Debug`.
hir::AssocItemConstraintKind::Bound { bounds: hir_bounds } => {
// NOTE: If `only_self_bounds` is true, do NOT expand this associated type bound into
// a trait predicate, since we only want to add predicates for the `Self` type.
if !only_self_bounds.0 {
let projection_ty = projection_term
.map_bound(|projection_term| projection_term.expect_ty(self.tcx()));
// Calling `skip_binder` is okay, because `lower_bounds` expects the `param_ty`
// parameter to have a skipped binder.
let param_ty = Ty::new_alias(tcx, ty::Projection, projection_ty.skip_binder());
self.lower_poly_bounds(
param_ty,
hir_bounds.iter(),
bounds,
projection_ty.bound_vars(),
only_self_bounds,
);
match predicate_filter {
PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => {}
PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => {
let projection_ty = projection_term
.map_bound(|projection_term| projection_term.expect_ty(self.tcx()));
// Calling `skip_binder` is okay, because `lower_bounds` expects the `param_ty`
// parameter to have a skipped binder.
let param_ty =
Ty::new_alias(tcx, ty::Projection, projection_ty.skip_binder());
self.lower_poly_bounds(
param_ty,
hir_bounds.iter(),
bounds,
projection_ty.bound_vars(),
predicate_filter,
);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use tracing::{debug, instrument};
use super::HirTyLowerer;
use crate::bounds::Bounds;
use crate::hir_ty_lowering::{
GenericArgCountMismatch, GenericArgCountResult, OnlySelfBounds, RegionInferReason,
GenericArgCountMismatch, GenericArgCountResult, PredicateFilter, RegionInferReason,
};

impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Expand Down Expand Up @@ -55,9 +55,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
ty::PredicatePolarity::Positive,
dummy_self,
&mut bounds,
// True so we don't populate `bounds` with associated type bounds, even
// though they're disallowed from object types.
OnlySelfBounds(true),
PredicateFilter::SelfOnly,
) {
potential_assoc_types.extend(cur_potential_assoc_types);
}
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ use crate::require_c_abi_if_c_variadic;
#[derive(Debug)]
pub struct GenericPathSegment(pub DefId, pub usize);

#[derive(Copy, Clone, Debug)]
pub struct OnlySelfBounds(pub bool);

#[derive(Copy, Clone, Debug)]
pub enum PredicateFilter {
/// All predicates may be implied by the trait.
Expand All @@ -76,7 +73,8 @@ pub enum PredicateFilter {
SelfOnly,

/// Only traits that reference `Self: ..` and define an associated type
/// with the given ident are implied by the trait.
/// with the given ident are implied by the trait. This mode exists to
/// side-step query cycles when lowering associated types.
SelfThatDefines(Ident),

/// Only traits that reference `Self: ..` and their associated type bounds.
Expand Down Expand Up @@ -683,7 +681,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
polarity: ty::PredicatePolarity,
self_ty: Ty<'tcx>,
bounds: &mut Bounds<'tcx>,
only_self_bounds: OnlySelfBounds,
predicate_filter: PredicateFilter,
) -> GenericArgCountResult {
let trait_def_id = trait_ref.trait_def_id().unwrap_or_else(|| FatalError.raise());
let trait_segment = trait_ref.path.segments.last().unwrap();
Expand Down Expand Up @@ -720,7 +718,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
span,
polarity,
constness,
only_self_bounds,
predicate_filter,
);

let mut dup_constraints = FxIndexMap::default();
Expand All @@ -744,7 +742,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
bounds,
&mut dup_constraints,
constraint.span,
only_self_bounds,
predicate_filter,
);
// Okay to ignore `Err` because of `ErrorGuaranteed` (see above).
}
Expand Down
Loading