Skip to content

Implement consecutive type-relative paths #126651

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
30 changes: 28 additions & 2 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! crate as a kind of pass. This should eventually be factored away.

use std::assert_matches::assert_matches;
use std::cell::Cell;
use std::cell::{Cell, RefCell};
use std::iter;
use std::ops::Bound;

Expand All @@ -34,6 +34,9 @@ use rustc_hir::{self as hir, GenericParamKind, HirId, Node, PreciseCapturingArgK
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_infer::traits::{DynCompatibilityViolation, ObligationCause};
use rustc_middle::query::Providers;
use rustc_middle::ty::typeck_results::{
HasTypeDependentDefs, LocalTableInContext, LocalTableInContextMut, TypeDependentDefs,
};
use rustc_middle::ty::util::{Discr, IntTypeExt};
use rustc_middle::ty::{
self, AdtKind, Const, IsSuggestable, Ty, TyCtxt, TypeVisitableExt, TypingMode, fold_regions,
Expand Down Expand Up @@ -132,6 +135,7 @@ pub(crate) fn provide(providers: &mut Providers) {
pub(crate) struct ItemCtxt<'tcx> {
tcx: TyCtxt<'tcx>,
item_def_id: LocalDefId,
type_dependent_defs: RefCell<TypeDependentDefs>,
tainted_by_errors: Cell<Option<ErrorGuaranteed>>,
}

Expand Down Expand Up @@ -243,7 +247,12 @@ fn bad_placeholder<'cx, 'tcx>(

impl<'tcx> ItemCtxt<'tcx> {
pub(crate) fn new(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId) -> ItemCtxt<'tcx> {
ItemCtxt { tcx, item_def_id, tainted_by_errors: Cell::new(None) }
ItemCtxt {
tcx,
item_def_id,
type_dependent_defs: Default::default(),
tainted_by_errors: Cell::new(None),
}
}

pub(crate) fn lower_ty(&self, hir_ty: &hir::Ty<'tcx>) -> Ty<'tcx> {
Expand Down Expand Up @@ -510,6 +519,14 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
// There's no place to record types from signatures?
}

fn record_res(&self, hir_id: hir::HirId, def_id: DefId) {
LocalTableInContextMut::new(
self.hir_id().owner,
&mut self.type_dependent_defs.borrow_mut(),
)
.insert(hir_id, Ok((self.tcx.def_kind(def_id), def_id)));
}

fn infcx(&self) -> Option<&InferCtxt<'tcx>> {
None
}
Expand Down Expand Up @@ -569,6 +586,15 @@ impl<'tcx> HirTyLowerer<'tcx> for ItemCtxt<'tcx> {
}
}

impl HasTypeDependentDefs for ItemCtxt<'_> {
fn type_dependent_def(&self, id: hir::HirId) -> Option<(DefKind, DefId)> {
LocalTableInContext::new(self.hir_id().owner, &self.type_dependent_defs.borrow())
.get(id)
.copied()
.and_then(|result| result.ok())
}
}

/// Synthesize a new lifetime name that doesn't clash with any of the lifetimes already present.
fn get_new_lifetime_name<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
matches!(args.parenthesized, hir::GenericArgsParentheses::ReturnTypeNotation)
}) =>
{
// FIXME(fmease): No longer true.
// First, ignore a qself that isn't a type or `Self` param. Those are the
// only ones that support `T::Assoc` anyways in HIR lowering.
let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = qself.kind else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
pub(super) fn report_unresolved_assoc_item<I>(
&self,
all_candidates: impl Fn() -> I,
qself: AssocItemQSelf,
qself: AssocItemQSelf<'tcx>,
assoc_tag: ty::AssocTag,
assoc_ident: Ident,
span: Span,
Expand Down
107 changes: 70 additions & 37 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ 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::typeck_results::HasTypeDependentDefs;
use rustc_middle::ty::{
self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, Ty, TyCtxt, TypeVisitableExt,
TypingMode, Upcast, fold_regions,
Expand Down Expand Up @@ -111,7 +112,7 @@ pub struct InherentAssocCandidate {
/// the [`rustc_middle::ty`] representation.
///
/// This trait used to be called `AstConv`.
pub trait HirTyLowerer<'tcx> {
pub trait HirTyLowerer<'tcx>: HasTypeDependentDefs {
fn tcx(&self) -> TyCtxt<'tcx>;

fn dcx(&self) -> DiagCtxtHandle<'_>;
Expand Down Expand Up @@ -202,6 +203,9 @@ pub trait HirTyLowerer<'tcx> {
/// Record the lowered type of a HIR node in this context.
fn record_ty(&self, hir_id: HirId, ty: Ty<'tcx>, span: Span);

/// Record the resolution of a HIR node corresponding to a type-dependent definition in this context.
fn record_res(&self, hir_id: hir::HirId, result: DefId);

/// The inference context of the lowering context if applicable.
fn infcx(&self) -> Option<&InferCtxt<'tcx>>;

Expand All @@ -224,18 +228,20 @@ pub trait HirTyLowerer<'tcx> {
/// The "qualified self" of an associated item path.
///
/// For diagnostic purposes only.
enum AssocItemQSelf {
enum AssocItemQSelf<'tcx> {
Trait(DefId),
TyParam(LocalDefId, Span),
SelfTyAlias,
AssocTy(Ty<'tcx>),
}

impl AssocItemQSelf {
fn to_string(&self, tcx: TyCtxt<'_>) -> String {
impl<'tcx> AssocItemQSelf<'tcx> {
fn to_string(&self, tcx: TyCtxt<'tcx>) -> String {
match *self {
Self::Trait(def_id) => tcx.def_path_str(def_id),
Self::TyParam(def_id, _) => tcx.hir_ty_param_name(def_id).to_string(),
Self::SelfTyAlias => kw::SelfUpper.to_string(),
Self::AssocTy(ty) => ty.to_string(),
}
}
}
Expand Down Expand Up @@ -1034,7 +1040,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
fn probe_single_bound_for_assoc_item<I>(
&self,
all_candidates: impl Fn() -> I,
qself: AssocItemQSelf,
qself: AssocItemQSelf<'tcx>,
assoc_tag: ty::AssocTag,
assoc_ident: Ident,
span: Span,
Expand Down Expand Up @@ -1153,27 +1159,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Lower a [type-relative](hir::QPath::TypeRelative) path in type position to a type.
///
/// If the path refers to an enum variant and `permit_variants` holds,
/// the returned type is simply the provided self type `qself_ty`.
///
/// A path like `A::B::C::D` is understood as `<A::B::C>::D`. I.e.,
/// `qself_ty` / `qself` is `A::B::C` and `assoc_segment` is `D`.
/// We return the lowered type and the `DefId` for the whole path.
/// the returned type is simply the provided self type `self_ty`.
///
/// We only support associated type paths whose self type is a type parameter or a `Self`
/// type alias (in a trait impl) like `T::Ty` (where `T` is a ty param) or `Self::Ty`.
/// We **don't** support paths whose self type is an arbitrary type like `Struct::Ty` where
/// struct `Struct` impls an in-scope trait that defines an associated type called `Ty`.
/// For the latter case, we report ambiguity.
/// While desirable to support, the implementation would be non-trivial. Tracked in [#22519].
///
/// At the time of writing, *inherent associated types* are also resolved here. This however
/// is [problematic][iat]. A proper implementation would be as non-trivial as the one
/// described in the previous paragraph and their modeling of projections would likely be
/// very similar in nature.
///
/// [#22519]: https://github.com/rust-lang/rust/issues/22519
/// [iat]: https://github.com/rust-lang/rust/issues/8995#issuecomment-1569208403
//
// NOTE: When this function starts resolving `Trait::AssocTy` successfully
// it should also start reporting the `BARE_TRAIT_OBJECTS` lint.
#[instrument(level = "debug", skip_all, ret)]
Expand All @@ -1185,7 +1172,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
qpath_hir_id: HirId,
span: Span,
permit_variants: PermitVariants,
) -> Result<(Ty<'tcx>, DefKind, DefId), ErrorGuaranteed> {
) -> Result<(Ty<'tcx>, DefId), ErrorGuaranteed> {
let tcx = self.tcx();
match self.lower_type_relative_path(
self_ty,
Expand All @@ -1198,11 +1185,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
TypeRelativePath::AssocItem(def_id, args) => {
let alias_ty = ty::AliasTy::new_from_args(tcx, def_id, args);
let ty = Ty::new_alias(tcx, alias_ty.kind(tcx), alias_ty);
Ok((ty, tcx.def_kind(def_id), def_id))
}
TypeRelativePath::Variant { adt, variant_did } => {
Ok((adt, DefKind::Variant, variant_did))
Ok((ty, def_id))
}
TypeRelativePath::Variant { adt, variant_did } => Ok((adt, variant_did)),
}
}

Expand Down Expand Up @@ -1246,6 +1231,27 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

/// Lower a [type-relative][hir::QPath::TypeRelative] (and type-level) path.
///
/// A path like `A::B::C::D` is understood as `<A::B::C>::D`. I.e., the self type is `A::B::C`
/// and the `segment` is `D`.
///
/// <!-- FIXME(fmease): No longer accurate -->
///
/// We only support associated item paths whose self type is a type parameter or a `Self`
/// type alias (in a trait impl) like `T::Ty` (where `T` is a ty param) or `Self::Ty`.
/// We **don't** support paths whose self type is an arbitrary type like `Struct::Ty` where
/// struct `Struct` impls an in-scope trait that defines an associated type called `Ty`.
/// For the latter case, we report ambiguity.
/// While desirable to support, the implementation would be non-trivial. Tracked in [#22519].
///
/// <!-- FIXME(fmease): Slightly outdated, too -->
///
/// At the time of writing, *inherent associated types* are also resolved here. This however
/// is problematic. A proper implementation would be as non-trivial as the one
/// described in the previous paragraph and their modeling of projections would likely be
/// very similar in nature.
///
/// [#22519]: https://github.com/rust-lang/rust/issues/22519
#[instrument(level = "debug", skip_all, ret)]
fn lower_type_relative_path(
&self,
Expand Down Expand Up @@ -1278,6 +1284,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
adt_def,
},
);
self.record_res(qpath_hir_id, variant_def.def_id);
return Ok(TypeRelativePath::Variant {
adt: self_ty,
variant_did: variant_def.def_id,
Expand All @@ -1289,15 +1296,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

// FIXME(inherent_associated_types, #106719): Support self types other than ADTs.
if let Some((did, args)) = self.probe_inherent_assoc_item(
if let Some((def_id, args)) = self.probe_inherent_assoc_item(
segment,
adt_def.did(),
self_ty,
qpath_hir_id,
span,
mode.assoc_tag(),
)? {
return Ok(TypeRelativePath::AssocItem(did, args));
self.record_res(qpath_hir_id, def_id);
return Ok(TypeRelativePath::AssocItem(def_id, args));
}
}

Expand Down Expand Up @@ -1360,7 +1368,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let tcx = self.tcx();

let self_ty_res = match hir_self_ty.kind {
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => path.res,
hir::TyKind::Path(qpath) => self.qpath_res(&qpath, hir_self_ty.hir_id),
_ => Res::Err,
};

Expand All @@ -1387,15 +1395,38 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
)?
}
(
&ty::Param(_),
Res::SelfTyParam { trait_: param_did } | Res::Def(DefKind::TyParam, param_did),
ty::Param(_),
Res::SelfTyParam { trait_: param_def_id }
| Res::Def(DefKind::TyParam, param_def_id),
Comment on lines +1399 to +1400
Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated: are there ever cases where we have a ty::Param with a different res? and if so, do we want to support them? we can go from the index of the param to its def_id after all

Copy link
Member

Choose a reason for hiding this comment

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

Self params dont resolve to DefKind::TyParam, they resolve to the trait defid and DefKind::Trait iirc

Copy link
Member

Choose a reason for hiding this comment

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

actually wait im blind thats SelfTyParam already

) => self.probe_single_ty_param_bound_for_assoc_item(
param_did.expect_local(),
param_def_id.expect_local(),
hir_self_ty.span,
assoc_tag,
segment.ident,
span,
)?,
(ty::Alias(ty::Projection, alias_ty), Res::Def(DefKind::AssocTy, _)) => {
// FIXME: Utilizing `item_bounds` for this is cycle-prone.
let predicates = tcx.item_bounds(alias_ty.def_id).instantiate(tcx, alias_ty.args);
Copy link
Contributor

Choose a reason for hiding this comment

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

does this cycle for

trait Foo {
    type FooAssoc;
}

trait Bar<T> {}

trait Trait {
    type Assoc: Foo + Bar<Self::Assoc::FooAssoc>;
} 

Copy link
Member Author

@fmease fmease Jul 1, 2024

Choose a reason for hiding this comment

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

Yeah, it does. I guess there's no way around introducing a specialized variant of item_bounds if at all possible (well, HIR ty lowering is notorious for following such an approach).

Copy link
Contributor

@lcnr lcnr Jul 2, 2024

Choose a reason for hiding this comment

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

that may work 🤔 an easier alternative may be to forbid the consecutive shorthand in item bounds and super traits. It's not great, but it does avoid the issue


self.probe_single_bound_for_assoc_item(
|| {
let trait_refs = predicates.iter().filter_map(|pred| {
pred.as_trait_clause().map(|t| t.map_bound(|t| t.trait_ref))
});
traits::transitive_bounds_that_define_assoc_item(
tcx,
trait_refs,
segment.ident,
)
},
AssocItemQSelf::AssocTy(self_ty),
assoc_tag,
segment.ident,
span,
None,
)?
}
_ => {
return Err(self.report_unresolved_type_relative_path(
self_ty,
Expand All @@ -1413,6 +1444,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
.probe_assoc_item(segment.ident, assoc_tag, qpath_hir_id, span, bound.def_id())
.expect("failed to find associated item");

self.record_res(qpath_hir_id, assoc_item.def_id);

Ok((assoc_item.def_id, bound))
}

Expand Down Expand Up @@ -2514,7 +2547,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
hir_ty.span,
PermitVariants::No,
)
.map(|(ty, _, _)| ty)
.map(|(ty, _)| ty)
.unwrap_or_else(|guar| Ty::new_error(tcx, guar))
}
&hir::TyKind::Path(hir::QPath::LangItem(lang_item, span)) => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(crate) fn write_resolution(
&self,
hir_id: HirId,
r: Result<(DefKind, DefId), ErrorGuaranteed>,
result: Result<(DefKind, DefId), ErrorGuaranteed>,
) {
self.typeck_results.borrow_mut().type_dependent_defs_mut().insert(hir_id, r);
self.typeck_results.borrow_mut().type_dependent_defs_mut().insert(hir_id, result);
}

#[instrument(level = "debug", skip(self))]
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2211,16 +2211,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
path_span,
PermitVariants::Yes,
);
let ty = result
.map(|(ty, _, _)| ty)
.unwrap_or_else(|guar| Ty::new_error(self.tcx(), guar));
let ty =
result.map(|(ty, _)| ty).unwrap_or_else(|guar| Ty::new_error(self.tcx(), guar));
let ty = LoweredTy::from_raw(self, path_span, ty);
let result = result.map(|(_, kind, def_id)| (kind, def_id));

// Write back the new resolution.
self.write_resolution(hir_id, result);

(result.map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id)), ty)
(
result.map_or(Res::Err, |(_, def_id)| {
Res::Def(self.tcx().def_kind(def_id), def_id)
}),
ty,
)
}
QPath::LangItem(lang_item, span) => {
let (res, ty) = self.resolve_lang_item_path(lang_item, span, hir_id);
Expand Down
Loading
Loading