Skip to content

Don't build ParamEnv and do trait solving in ItemCtxts when lowering IATs #140247

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+643 −270
Open

Don't build ParamEnv and do trait solving in ItemCtxts when lowering IATs #140247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0cc9ca
Don't build `ParamEnv` and do trait solving in `ItemCtxt`s
BoxyUwU Apr 24, 2025
8f33b8f
Test whether we use DeepRejectCtxt
BoxyUwU May 27, 2025
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
72 changes: 69 additions & 3 deletions compiler/rustc_hir_analysis/src/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,20 @@ use rustc_infer::traits::ObligationCause;
use rustc_middle::hir::nested_filter;
use rustc_middle::query::Providers;
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypingMode, fold_regions};
use rustc_middle::ty::{
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt, TypingMode, fold_regions,
};
use rustc_middle::{bug, span_bug};
use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym};
use rustc_trait_selection::error_reporting::traits::suggestions::NextTypeParamName;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::ObligationCtxt;
use rustc_trait_selection::traits::{FulfillmentError, ObligationCtxt};
use tracing::{debug, instrument};

use crate::errors;
use crate::hir_ty_lowering::{FeedConstTy, HirTyLowerer, RegionInferReason};
use crate::hir_ty_lowering::{
FeedConstTy, HirTyLowerer, InherentAssocCandidate, RegionInferReason,
};

pub(crate) mod dump;
mod generics_of;
Expand Down Expand Up @@ -444,6 +448,68 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
self.tcx.at(span).type_param_predicates((self.item_def_id, def_id, assoc_ident))
}

#[instrument(level = "debug", skip(self, _span), ret)]
fn select_inherent_assoc_candidates(
&self,
_span: Span,
self_ty: Ty<'tcx>,
candidates: Vec<InherentAssocCandidate>,
) -> (Vec<InherentAssocCandidate>, Vec<FulfillmentError<'tcx>>) {
assert!(!self_ty.has_infer());

// We don't just call the normal normalization routine here as we can't provide the
// correct `ParamEnv` and it seems dubious to invoke arbitrary trait solving under
// the wrong `ParamEnv`. Expanding free aliases doesn't need a `ParamEnv` so we do
// this just to make resolution a little bit smarter.
let self_ty = self.tcx.expand_free_alias_tys(self_ty);
debug!("select_inherent_assoc_candidates: self_ty={:?}", self_ty);

// We make an infcx and replace any escaping vars with placeholders so that IAT res
// with type/const bound vars in arguments is slightly smarter. `for<T> <Foo<T>>::IAT`
// would otherwise unify with `impl Foo<u8>` when ideally we would not.
let infcx = self.tcx().infer_ctxt().build(TypingMode::non_body_analysis());
Copy link
Member Author

Choose a reason for hiding this comment

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

Creating an infcx just to make placeholders feels pretty bad lol

Copy link
Contributor

Choose a reason for hiding this comment

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

do u need to? id expect expand free aliases to handle escaping bound vars. so does deep reject

Copy link
Member Author

@BoxyUwU BoxyUwU May 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

deep reject "handles" escaping vars by allowing them to unify with anything, which is weaker IAT resolution than replacing with placeholders. See tests/ui/associated-inherent-types/bound_vars_in_args.rs which fails if we don't replace with placeholders

let mut universes = if self_ty.has_escaping_bound_vars() {
vec![None; self_ty.outer_exclusive_binder().as_usize()]
} else {
vec![]
};
let candidates =
rustc_trait_selection::traits::with_replaced_escaping_bound_vars(
&infcx,
&mut universes,
self_ty,
|self_ty| {
candidates
.into_iter()
.filter(|&InherentAssocCandidate { impl_, .. }| {
let impl_ty = self.tcx().type_of(impl_).instantiate_identity();

// See comment on doing this operation for `self_ty`
let impl_ty = self.tcx.expand_free_alias_tys(impl_ty);
debug!("select_inherent_assoc_candidates: impl_ty={:?}", impl_ty);

// We treat parameters in the self ty as rigid and parameters in the impl ty as infers
// because it allows `impl<T> Foo<T>` to unify with `Foo<u8>::IAT`, while also disallowing
// `Foo<T>::IAT` from unifying with `impl Foo<u8>`.
//
// We don't really care about a depth limit here because we're only working with user-written
// types and if they wrote a type that would take hours to walk then that's kind of on them. On
// the other hand the default depth limit is relatively low and could realistically be hit by
// users in normal cases.
//
// `DeepRejectCtxt` leads to slightly worse IAT resolution than real type equality in cases
// where the `impl_ty` has repeated uses of generic parameters. E.g. `impl<T> Foo<T, T>` would
// be considered a valid candidate when resolving `Foo<u8, u16>::IAT`.
ty::DeepRejectCtxt::relate_rigid_infer(self.tcx)
.types_may_unify_with_depth(self_ty, impl_ty, usize::MAX)
})
.collect()
},
);

(candidates, vec![])
}

fn lower_assoc_item_path(
&self,
span: Span,
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use rustc_trait_selection::traits::{
use smallvec::SmallVec;
use tracing::debug;

use super::InherentAssocCandidate;
use crate::errors::{
self, AssocItemConstraintsNotAllowedHere, ManualImplementation, MissingTypeParams,
ParenthesizedFnTraitExpansion, TraitObjectDeclaredWithNoTraits,
Expand Down Expand Up @@ -742,7 +743,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
&self,
name: Ident,
self_ty: Ty<'tcx>,
candidates: Vec<(DefId, (DefId, DefId))>,
candidates: Vec<InherentAssocCandidate>,
fulfillment_errors: Vec<FulfillmentError<'tcx>>,
span: Span,
assoc_tag: ty::AssocTag,
Expand Down Expand Up @@ -776,8 +777,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let type_candidates = candidates
.iter()
.take(limit)
.map(|&(impl_, _)| {
format!("- `{}`", tcx.at(span).type_of(impl_).instantiate_identity())
.map(|cand| {
format!("- `{}`", tcx.at(span).type_of(cand.impl_).instantiate_identity())
})
.collect::<Vec<_>>()
.join("\n");
Expand Down
150 changes: 38 additions & 112 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{self as hir, AnonConst, GenericArg, GenericArgs, HirId};
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_infer::traits::ObligationCause;
use rustc_macros::{TypeFoldable, TypeVisitable};
use rustc_middle::middle::stability::AllowUnstable;
use rustc_middle::mir::interpret::LitToConstInput;
use rustc_middle::ty::print::PrintPolyTraitRefExt as _;
use rustc_middle::ty::{
self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, ParamEnv, Ty, TyCtxt,
TypeVisitableExt, TypingMode, Upcast, fold_regions,
self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, Ty, TyCtxt, TypeVisitableExt,
TypingMode, Upcast, fold_regions,
};
use rustc_middle::{bug, span_bug};
use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS;
use rustc_session::parse::feature_err;
use rustc_span::{DUMMY_SP, Ident, Span, kw, sym};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::wf::object_region_bounds;
use rustc_trait_selection::traits::{self, ObligationCtxt};
use rustc_trait_selection::traits::{self, FulfillmentError};
use tracing::{debug, instrument};

use crate::check::check_abi_fn_ptr;
Expand Down Expand Up @@ -99,6 +99,13 @@ pub enum RegionInferReason<'a> {
OutlivesBound,
}

#[derive(Copy, Clone, TypeFoldable, TypeVisitable, Debug)]
pub struct InherentAssocCandidate {
pub impl_: DefId,
pub assoc_item: DefId,
pub scope: DefId,
}

/// A context which can lower type-system entities from the [HIR][hir] to
/// the [`rustc_middle::ty`] representation.
///
Expand Down Expand Up @@ -148,6 +155,13 @@ pub trait HirTyLowerer<'tcx> {
assoc_ident: Ident,
) -> ty::EarlyBinder<'tcx, &'tcx [(ty::Clause<'tcx>, Span)]>;

fn select_inherent_assoc_candidates(
&self,
span: Span,
self_ty: Ty<'tcx>,
candidates: Vec<InherentAssocCandidate>,
) -> (Vec<InherentAssocCandidate>, Vec<FulfillmentError<'tcx>>);

/// Lower a path to an associated item (of a trait) to a projection.
///
/// This method has to be defined by the concrete lowering context because
Expand Down Expand Up @@ -1441,48 +1455,32 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
.filter_map(|&impl_| {
let (item, scope) =
self.probe_assoc_item_unchecked(name, assoc_tag, block, impl_)?;
Some((impl_, (item.def_id, scope)))
Some(InherentAssocCandidate { impl_, assoc_item: item.def_id, scope })
})
.collect();

if candidates.is_empty() {
return Ok(None);
}
Comment on lines -1448 to -1450
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this masked a bunch of mgca ICEs I think. We now emit errors in all cases where IAT resolution fails to find any candidates which makes some delayed bugs no longer trigger. I've replaced a few of the crashes tests with examples that don't rely on IATs and I removed the two others that I couldn't find a reproducer for that didn't seem like a dupe of the other three.


//
// Select applicable inherent associated type candidates modulo regions.
//

// In contexts that have no inference context, just make a new one.
// We do need a local variable to store it, though.
let infcx = match self.infcx() {
Some(infcx) => infcx,
None => {
assert!(!self_ty.has_infer());
&tcx.infer_ctxt().ignoring_regions().build(TypingMode::non_body_analysis())
}
};
let (applicable_candidates, fulfillment_errors) =
self.select_inherent_assoc_candidates(span, self_ty, candidates.clone());

// FIXME(inherent_associated_types): Acquiring the ParamEnv this early leads to cycle errors
// when inside of an ADT (#108491) or where clause.
let param_env = tcx.param_env(block.owner);
let InherentAssocCandidate { impl_, assoc_item, scope: def_scope } =
match &applicable_candidates[..] {
&[] => Err(self.report_unresolved_inherent_assoc_item(
name,
self_ty,
candidates,
fulfillment_errors,
span,
assoc_tag,
)),

let mut universes = if self_ty.has_escaping_bound_vars() {
vec![None; self_ty.outer_exclusive_binder().as_usize()]
} else {
vec![]
};
&[applicable_candidate] => Ok(applicable_candidate),

let (impl_, (assoc_item, def_scope)) = crate::traits::with_replaced_escaping_bound_vars(
infcx,
&mut universes,
self_ty,
|self_ty| {
self.select_inherent_assoc_candidates(
infcx, name, span, self_ty, param_env, candidates, assoc_tag,
)
},
)?;
&[_, ..] => Err(self.report_ambiguous_inherent_assoc_item(
name,
candidates.into_iter().map(|cand| cand.assoc_item).collect(),
span,
)),
}?;

self.check_assoc_item(assoc_item, name, def_scope, block, span);

Expand All @@ -1499,78 +1497,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Ok(Some((assoc_item, args)))
}

fn select_inherent_assoc_candidates(
&self,
infcx: &InferCtxt<'tcx>,
name: Ident,
span: Span,
self_ty: Ty<'tcx>,
param_env: ParamEnv<'tcx>,
candidates: Vec<(DefId, (DefId, DefId))>,
assoc_tag: ty::AssocTag,
) -> Result<(DefId, (DefId, DefId)), ErrorGuaranteed> {
let tcx = self.tcx();
let mut fulfillment_errors = Vec::new();

let applicable_candidates: Vec<_> = candidates
.iter()
.copied()
.filter(|&(impl_, _)| {
infcx.probe(|_| {
let ocx = ObligationCtxt::new_with_diagnostics(infcx);
let self_ty = ocx.normalize(&ObligationCause::dummy(), param_env, self_ty);

let impl_args = infcx.fresh_args_for_item(span, impl_);
let impl_ty = tcx.type_of(impl_).instantiate(tcx, impl_args);
let impl_ty = ocx.normalize(&ObligationCause::dummy(), param_env, impl_ty);

// Check that the self types can be related.
if ocx.eq(&ObligationCause::dummy(), param_env, impl_ty, self_ty).is_err() {
return false;
}

// Check whether the impl imposes obligations we have to worry about.
let impl_bounds = tcx.predicates_of(impl_).instantiate(tcx, impl_args);
let impl_bounds =
ocx.normalize(&ObligationCause::dummy(), param_env, impl_bounds);
let impl_obligations = traits::predicates_for_generics(
|_, _| ObligationCause::dummy(),
param_env,
impl_bounds,
);
ocx.register_obligations(impl_obligations);

let mut errors = ocx.select_where_possible();
if !errors.is_empty() {
fulfillment_errors.append(&mut errors);
return false;
}

true
})
})
.collect();

match &applicable_candidates[..] {
&[] => Err(self.report_unresolved_inherent_assoc_item(
name,
self_ty,
candidates,
fulfillment_errors,
span,
assoc_tag,
)),

&[applicable_candidate] => Ok(applicable_candidate),

&[_, ..] => Err(self.report_ambiguous_inherent_assoc_item(
name,
applicable_candidates.into_iter().map(|(_, (candidate, _))| candidate).collect(),
span,
)),
}
}

/// Given name and kind search for the assoc item in the provided scope and check if it's accessible[^1].
///
/// [^1]: I.e., accessible in the provided scope wrt. visibility and stability.
Expand Down
Loading
Loading