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

Introduce AliasKind::Inherent for inherent associated types #109410

Merged
merged 4 commits into from
May 8, 2023
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
61 changes: 35 additions & 26 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2378,6 +2378,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
return Ok(None);
}

//
// 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_;
Expand All @@ -2390,14 +2394,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
};

let param_env = tcx.param_env(block.owner.to_def_id());
// 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 cause = ObligationCause::misc(span, block.owner.def_id);

let mut fulfillment_errors = Vec::new();
let mut applicable_candidates: Vec<_> = infcx.probe(|_| {
let universe = infcx.create_next_universe();

// Regions are not considered during selection.
// FIXME(non_lifetime_binders): Here we are "truncating" or "flattening" the universes
// of type and const binders. Is that correct in the selection phase? See also #109505.
let self_ty = tcx.replace_escaping_bound_vars_uncached(
self_ty,
FnMutDelegate {
Expand All @@ -2413,41 +2421,40 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

candidates
.iter()
.filter_map(|&(impl_, (assoc_item, def_scope))| {
.copied()
.filter(|&(impl_, _)| {
infcx.probe(|_| {
let ocx = ObligationCtxt::new_in_snapshot(&infcx);

let impl_ty = tcx.type_of(impl_);
let impl_substs = infcx.fresh_item_substs(impl_);
let impl_ty = impl_ty.subst(tcx, impl_substs);
let impl_ty = tcx.type_of(impl_).subst(tcx, impl_substs);
let impl_ty = ocx.normalize(&cause, param_env, impl_ty);

// Check that the Self-types can be related.
// FIXME(fmease): Should we use `eq` here?
ocx.sup(&ObligationCause::dummy(), param_env, impl_ty, self_ty).ok()?;
// Check that the self types can be related.
// FIXME(inherent_associated_types): Should we use `eq` here? Method probing uses
// `sup` for this situtation, too. What for? To constrain inference variables?
if ocx.sup(&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_);
let impl_bounds = impl_bounds.instantiate(tcx, impl_substs);

let impl_bounds = tcx.predicates_of(impl_).instantiate(tcx, impl_substs);
let impl_bounds = ocx.normalize(&cause, param_env, impl_bounds);

let impl_obligations = traits::predicates_for_generics(
|_, _| cause.clone(),
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 None;
return false;
}

// FIXME(fmease): Unsolved vars can escape this InferCtxt snapshot.
Some((assoc_item, def_scope, infcx.resolve_vars_if_possible(impl_substs)))
true
})
})
.collect()
Expand All @@ -2456,24 +2463,26 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
if applicable_candidates.len() > 1 {
return Err(self.complain_about_ambiguous_inherent_assoc_type(
name,
applicable_candidates.into_iter().map(|(candidate, ..)| candidate).collect(),
applicable_candidates.into_iter().map(|(_, (candidate, _))| candidate).collect(),
span,
));
}

if let Some((assoc_item, def_scope, impl_substs)) = applicable_candidates.pop() {
if let Some((impl_, (assoc_item, def_scope))) = applicable_candidates.pop() {
self.check_assoc_ty(assoc_item, name, def_scope, block, span);

// FIXME(inherent_associated_types): To fully *confirm* the *probed* candidate, we still
// need to relate the Self-type with fresh item substs & register region obligations for
// regionck to prove/disprove.

let item_substs =
self.create_substs_for_associated_item(span, assoc_item, segment, impl_substs);
// FIXME(fmease): Currently creating throwaway `parent_substs` to please
Copy link
Member Author

Choose a reason for hiding this comment

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

If you would like me to, I can address this in this by PR by changing the API. It has several users though.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine for now.

// `create_substs_for_associated_item`. Modify the latter instead (or sth. similar) to
// not require the parent substs logic.
let parent_substs = InternalSubsts::identity_for_item(tcx, impl_);
let substs =
self.create_substs_for_associated_item(span, assoc_item, segment, parent_substs);
let substs = tcx.mk_substs_from_iter(
std::iter::once(ty::GenericArg::from(self_ty))
.chain(substs.into_iter().skip(parent_substs.len())),
);

// FIXME(fmease, #106722): Check if the bounds on the parameters of the
// associated type hold, if any.
let ty = tcx.type_of(assoc_item).subst(tcx, item_substs);
let ty = tcx.mk_alias(ty::Inherent, tcx.mk_alias_ty(assoc_item, substs));

return Ok(Some((ty, assoc_item)));
}
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_hir_analysis/src/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,19 @@ fn do_orphan_check_impl<'tcx>(
NonlocalImpl::DisallowOther,
),

// ```
// struct S<T>(T);
// impl<T: ?Sized> S<T> {
// type This = T;
// }
// impl<T: ?Sized> AutoTrait for S<T>::This {}
fmease marked this conversation as resolved.
Show resolved Hide resolved
// ```
// FIXME(inherent_associated_types): The example code above currently leads to a cycle
ty::Alias(AliasKind::Inherent, _) => (
LocalImpl::Disallow { problematic_kind: "associated type" },
NonlocalImpl::DisallowOther,
),

// type Opaque = impl Trait;
// impl AutoTrait for Opaque {}
ty::Alias(AliasKind::Opaque, _) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1938,7 +1938,7 @@ fn is_late_bound_map(
ty::Param(param_ty) => {
self.arg_is_constrained[param_ty.index as usize] = true;
}
ty::Alias(ty::Projection, _) => return ControlFlow::Continue(()),
ty::Alias(ty::Projection | ty::Inherent, _) => return ControlFlow::Continue(()),
_ => (),
}
t.super_visit_with(self)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx> {
// the def_id that this query was called with. We filter to only type and const args here
// as a precaution for if it's ever allowed to elide lifetimes in GAT's. It currently isn't
// but it can't hurt to be safe ^^
if let ty::Alias(ty::Projection, projection) = ty.kind() {
if let ty::Alias(ty::Projection | ty::Inherent, projection) = ty.kind() {
let generics = tcx.generics_of(projection.def_id);

let arg_index = segment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct ParameterCollector {
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ParameterCollector {
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
match *t.kind() {
ty::Alias(ty::Projection, ..) if !self.include_nonconstraining => {
ty::Alias(ty::Projection | ty::Inherent, ..) if !self.include_nonconstraining => {
// projections are not injective
return ControlFlow::Continue(());
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ fn insert_required_predicates_to_be_wf<'tcx>(
);
}

// FIXME(inherent_associated_types): Handle this case properly.
ty::Alias(ty::Inherent, _) => {}
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to think harder about this one.

Copy link
Member

Choose a reason for hiding this comment

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

Not blocking I guess.


_ => {}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ fn find_param_in_ty<'tcx>(
return true;
}
if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Alias(ty::Projection, ..) = ty.kind()
&& let ty::Alias(ty::Projection | ty::Inherent, ..) = ty.kind()
{
// This logic may seem a bit strange, but typically when
// we have a projection type in a function signature, the
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
match ty.kind() {
ty::Adt(adt_def, _) => Some(*adt_def),
// FIXME(#104767): Should we handle bound regions here?
ty::Alias(ty::Projection, _) if !ty.has_escaping_bound_vars() => {
ty::Alias(ty::Projection | ty::Inherent, _) if !ty.has_escaping_bound_vars() => {
self.normalize(span, ty).ty_adt_def()
}
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2211,7 +2211,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
| ty::Float(_)
| ty::Adt(_, _)
| ty::Str
| ty::Alias(ty::Projection, _)
| ty::Alias(ty::Projection | ty::Inherent, _)
| ty::Param(_) => format!("{deref_ty}"),
// we need to test something like <&[_]>::len or <(&[u32])>::len
// and Vec::function();
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ impl<'tcx> InferCtxt<'tcx> {
bug!()
}

(_, ty::Alias(AliasKind::Projection, _)) | (ty::Alias(AliasKind::Projection, _), _)
(_, ty::Alias(AliasKind::Projection | AliasKind::Inherent, _))
Copy link
Member

Choose a reason for hiding this comment

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

Yes, technically, though I'm not sure if the new solver is adjusted to handle projection goals with IATs, so it'll ICE with -Ztrait-solver=next.

| (ty::Alias(AliasKind::Projection | AliasKind::Inherent, _), _)
if self.tcx.trait_solver_next() =>
{
relation.register_type_relate_obligation(a, b);
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2354,7 +2354,9 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
let labeled_user_string = match bound_kind {
GenericKind::Param(ref p) => format!("the parameter type `{}`", p),
GenericKind::Alias(ref p) => match p.kind(self.tcx) {
ty::AliasKind::Projection => format!("the associated type `{}`", p),
ty::AliasKind::Projection | ty::AliasKind::Inherent => {
format!("the associated type `{}`", p)
}
ty::AliasKind::Opaque => format!("the opaque type `{}`", p),
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
#traits-as-parameters",
);
}
(ty::Alias(ty::Projection, _), ty::Alias(ty::Projection, _)) => {
(ty::Alias(ty::Projection | ty::Inherent, _), ty::Alias(ty::Projection | ty::Inherent, _)) => {
diag.note("an associated type was expected, but a different one was found");
}
// FIXME(inherent_associated_types): Extend this to support `ty::Inherent`, too.
(ty::Param(p), ty::Alias(ty::Projection, proj)) | (ty::Alias(ty::Projection, proj), ty::Param(p))
if !tcx.is_impl_trait_in_trait(proj.def_id) =>
{
Expand Down Expand Up @@ -222,7 +223,7 @@ impl<T> Trait<T> for X {
diag.span_label(p_span, "this type parameter");
}
}
(ty::Alias(ty::Projection, proj_ty), _) if !tcx.is_impl_trait_in_trait(proj_ty.def_id) => {
(ty::Alias(ty::Projection | ty::Inherent, proj_ty), _) if !tcx.is_impl_trait_in_trait(proj_ty.def_id) => {
self.expected_projection(
diag,
proj_ty,
Expand All @@ -231,7 +232,7 @@ impl<T> Trait<T> for X {
cause.code(),
);
}
(_, ty::Alias(ty::Projection, proj_ty)) if !tcx.is_impl_trait_in_trait(proj_ty.def_id) => {
(_, ty::Alias(ty::Projection | ty::Inherent, proj_ty)) if !tcx.is_impl_trait_in_trait(proj_ty.def_id) => {
let msg = format!(
"consider constraining the associated type `{}` to `{}`",
values.found, values.expected,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ impl<'tcx> InferCtxt<'tcx> {
// We can't normalize associated types from `rustc_infer`,
// but we can eagerly register inference variables for them.
// FIXME(RPITIT): Don't replace RPITITs with inference vars.
// FIXME(inherent_associated_types): Extend this to support `ty::Inherent`, too.
ty::Alias(ty::Projection, projection_ty)
if !projection_ty.has_escaping_bound_vars()
&& !tcx.is_impl_trait_in_trait(projection_ty.def_id) =>
Expand All @@ -569,6 +570,7 @@ impl<'tcx> InferCtxt<'tcx> {
hidden_ty
}
// FIXME(RPITIT): This can go away when we move to associated types
// FIXME(inherent_associated_types): Extend this to support `ty::Inherent`, too.
ty::Alias(
ty::Projection,
ty::AliasTy { def_id: def_id2, substs: substs2, .. },
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::layout::{LayoutError, LayoutOf};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef};
use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -1461,6 +1462,10 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
// Bounds are respected for `type X = impl Trait`
return;
}
if cx.tcx.type_of(item.owner_id).skip_binder().has_inherent_projections() {
Copy link
Member

Choose a reason for hiding this comment

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

hmm... why?

Copy link
Member Author

@fmease fmease Mar 21, 2023

Choose a reason for hiding this comment

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

Why this deep check for inherent projections? Well, bounds on type aliases are respected as matter of fact if the RHS contains an IAT since we look at the ParamEnv of the type alias during selection in astconv to be able to successfully construct a Ty. If I didn't do this check here, the lint would misfire / claim a falsehood.

In "IAT, take 3" that we briefly talked about, with a hypothetical AliasKind::InherentCandidate we could delay selection and probably wouldn't need to respect the bounds of type aliases. Maybe there's a fourth solution, too.

Note that the AliasKind::Weak PR does basically the same thing. Outside of that, ty alias bounds are already respected for opaques, e.g. type A<T: B> = impl Tr<T>, so it's nothing too controversial (?).

// Bounds are respected for `type X = … Type::Inherent …`
return;
}
// There must not be a where clause
if type_alias_generics.predicates.is_empty() {
return;
Expand Down Expand Up @@ -1580,7 +1585,6 @@ declare_lint_pass!(

impl<'tcx> LateLintPass<'tcx> for TrivialConstraints {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::Clause;
use rustc_middle::ty::PredicateKind::*;

Expand Down Expand Up @@ -2917,6 +2921,7 @@ impl ClashingExternDeclarations {
| (Generator(..), Generator(..))
| (GeneratorWitness(..), GeneratorWitness(..))
| (Alias(ty::Projection, ..), Alias(ty::Projection, ..))
| (Alias(ty::Inherent, ..), Alias(ty::Inherent, ..))
| (Alias(ty::Opaque, ..), Alias(ty::Opaque, ..)) => false,

// These definitely should have been caught above.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,14 +1119,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

// `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe,
// so they are currently ignored for the purposes of this lint.
ty::Param(..) | ty::Alias(ty::Projection, ..)
ty::Param(..) | ty::Alias(ty::Projection | ty::Inherent, ..)
if matches!(self.mode, CItemKind::Definition) =>
{
FfiSafe
}

ty::Param(..)
| ty::Alias(ty::Projection, ..)
| ty::Alias(ty::Projection | ty::Inherent, ..)
| ty::Infer(..)
| ty::Bound(..)
| ty::Error(_)
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1821,6 +1821,16 @@ rustc_queries! {
desc { "normalizing `{}`", goal.value.value }
}

/// Do not call this query directly: invoke `normalize` instead.
query normalize_inherent_projection_ty(
goal: CanonicalProjectionGoal<'tcx>
) -> Result<
&'tcx Canonical<'tcx, canonical::QueryResponse<'tcx, NormalizationResult<'tcx>>>,
NoSolution,
> {
desc { "normalizing `{}`", goal.value.value }
}

/// Do not call this query directly: invoke `try_normalize_erasing_regions` instead.
query try_normalize_generic_arg_after_erasing_regions(
goal: ParamEnvAnd<'tcx, GenericArg<'tcx>>
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,17 @@ impl<'tcx> TyCtxt<'tcx> {
let substs = substs.into_iter().map(Into::into);
#[cfg(debug_assertions)]
{
let n = self.generics_of(_def_id).count();
let generics = self.generics_of(_def_id);

let n = if let DefKind::AssocTy = self.def_kind(_def_id)
&& let DefKind::Impl { of_trait: false } = self.def_kind(self.parent(_def_id))
{
// If this is an inherent projection.

generics.params.len() + 1
} else {
generics.count()
};
assert_eq!(
(n, Some(n)),
substs.size_hint(),
Expand Down Expand Up @@ -2007,7 +2017,7 @@ impl<'tcx> TyCtxt<'tcx> {
debug_assert_matches!(
(kind, self.def_kind(alias_ty.def_id)),
(ty::Opaque, DefKind::OpaqueTy)
| (ty::Projection, DefKind::AssocTy)
| (ty::Projection | ty::Inherent, DefKind::AssocTy)
| (ty::Opaque | ty::Projection, DefKind::ImplTraitPlaceholder)
);
self.mk_ty_from_kind(Alias(kind, alias_ty))
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl<'tcx> Ty<'tcx> {
ty::Infer(ty::FreshTy(_)) => "fresh type".into(),
ty::Infer(ty::FreshIntTy(_)) => "fresh integral type".into(),
ty::Infer(ty::FreshFloatTy(_)) => "fresh floating-point type".into(),
ty::Alias(ty::Projection, _) => "associated type".into(),
ty::Alias(ty::Projection | ty::Inherent, _) => "associated type".into(),
ty::Param(p) => format!("type parameter `{p}`").into(),
ty::Alias(ty::Opaque, ..) => if tcx.ty_is_opaque_future(self) { "future".into() } else { "opaque type".into() },
ty::Error(_) => "type error".into(),
Expand Down Expand Up @@ -312,7 +312,7 @@ impl<'tcx> Ty<'tcx> {
ty::Tuple(..) => "tuple".into(),
ty::Placeholder(..) => "higher-ranked type".into(),
ty::Bound(..) => "bound type variable".into(),
ty::Alias(ty::Projection, _) => "associated type".into(),
ty::Alias(ty::Projection | ty::Inherent, _) => "associated type".into(),
ty::Param(_) => "type parameter".into(),
ty::Alias(ty::Opaque, ..) => "opaque type".into(),
}
Expand Down
Loading