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

Install projection from RPITIT to default trait method opaque correctly #109198

Merged
merged 9 commits into from
Mar 17, 2023
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ fn check_item_type(tcx: TyCtxt<'_>, id: hir::ItemId) {
check_opaque(tcx, id);
}
DefKind::ImplTraitPlaceholder => {
let parent = tcx.impl_trait_in_trait_parent(id.owner_id.to_def_id());
let parent = tcx.impl_trait_in_trait_parent_fn(id.owner_id.to_def_id());
// Only check the validity of this opaque type if the function has a default body
if let hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(_, hir::TraitFn::Provided(_)),
Expand Down
24 changes: 17 additions & 7 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::autoderef::Autoderef;
use crate::constrained_generic_params::{identify_constrained_generic_params, Parameter};

use hir::def::DefKind;
use rustc_ast as ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed};
Expand Down Expand Up @@ -1546,16 +1545,27 @@ fn check_return_position_impl_trait_in_trait_bounds<'tcx>(
if let Some(assoc_item) = tcx.opt_associated_item(fn_def_id.to_def_id())
&& assoc_item.container == ty::AssocItemContainer::TraitContainer
{
// FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty): Even with the new lowering
// strategy, we can't just call `check_associated_item` on the new RPITITs,
// because tests like `tests/ui/async-await/in-trait/implied-bounds.rs` will fail.
// That's because we need to check that the bounds of the RPITIT hold using
// the special substs that we create during opaque type lowering, otherwise we're
// getting a bunch of early bound and free regions mixed up... Haven't looked too
// deep into this, though.
for arg in fn_output.walk() {
if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Alias(ty::Opaque, proj) = ty.kind()
&& tcx.def_kind(proj.def_id) == DefKind::ImplTraitPlaceholder
&& tcx.impl_trait_in_trait_parent(proj.def_id) == fn_def_id.to_def_id()
// RPITITs are always eagerly normalized into opaques, so always look for an
// opaque here.
&& let ty::Alias(ty::Opaque, opaque_ty) = ty.kind()
&& let Some(opaque_def_id) = opaque_ty.def_id.as_local()
&& let opaque = tcx.hir().expect_item(opaque_def_id).expect_opaque_ty()
&& let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = opaque.origin
&& source == fn_def_id
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correct this is equivalent to what was before but done a little bit better, or are there actual difference? if so in which cases?.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not equivalent. If we call is_impl_trait_in_trait on an opaque def id, then it returns false.

Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, some tests begin to spuriously pass wfcheck.

Copy link
Member Author

@compiler-errors compiler-errors Mar 16, 2023

Choose a reason for hiding this comment

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

the first commit in the stack "breaks" tests/ui/impl-trait/in-trait/wf-bounds.rs by actually projecting the RPITIT to a real opaque type, so this commit fixes the test by always checking for an opaque type.

Copy link
Member

Choose a reason for hiding this comment

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

the first commit in the stack "breaks" tests/ui/impl-trait/in-trait/wf-bounds.rs by actually projecting the RPITIT to a real opaque type, so this commit fixes the test by always checking for an opaque type.

Ahh right, because we project the projection to an opaque, that's the thing.

{
let span = tcx.def_span(proj.def_id);
let bounds = wfcx.tcx().explicit_item_bounds(proj.def_id);
let span = tcx.def_span(opaque_ty.def_id);
let bounds = wfcx.tcx().explicit_item_bounds(opaque_ty.def_id);
let wf_obligations = bounds.iter().flat_map(|&(bound, bound_span)| {
let bound = ty::EarlyBinder(bound).subst(tcx, proj.substs);
let bound = ty::EarlyBinder(bound).subst(tcx, opaque_ty.substs);
let normalized_bound = wfcx.normalize(span, None, bound);
traits::wf::predicate_obligations(
wfcx.infcx,
Expand Down
36 changes: 22 additions & 14 deletions compiler/rustc_hir_analysis/src/collect/item_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::astconv::AstConv;
use rustc_hir as hir;
use rustc_infer::traits::util;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::{self, ImplTraitInTraitData, Ty, TyCtxt};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::def_id::DefId;
use rustc_span::Span;

Expand Down Expand Up @@ -76,18 +76,26 @@ pub(super) fn explicit_item_bounds(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> &'_ [(ty::Predicate<'_>, Span)] {
// If the def_id is about an RPITIT, delegate explicit_item_bounds to the opaque_def_id that
// generated the synthesized associate type.
let rpitit_info = if let Some(ImplTraitInTraitData::Trait { opaque_def_id, .. }) =
tcx.opt_rpitit_info(def_id)
{
Some(opaque_def_id)
} else {
None
};
match tcx.opt_rpitit_info(def_id) {
// RPITIT's bounds are the same as opaque type bounds, but with
// a projection self type.
Some(ty::ImplTraitInTraitData::Trait { opaque_def_id, .. }) => {
let item = tcx.hir().get_by_def_id(opaque_def_id.expect_local()).expect_item();
let opaque_ty = item.expect_opaque_ty();
return opaque_type_bounds(
tcx,
opaque_def_id,
opaque_ty.bounds,
tcx.mk_projection(def_id, ty::InternalSubsts::identity_for_item(tcx, def_id)),
item.span,
);
}
// These should have been fed!
Some(ty::ImplTraitInTraitData::Impl { .. }) => unreachable!(),
None => {}
}

let bounds_def_id = rpitit_info.unwrap_or(def_id);
let hir_id = tcx.hir().local_def_id_to_hir_id(bounds_def_id.expect_local());
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
match tcx.hir().get(hir_id) {
hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Type(bounds, _),
Expand All @@ -100,12 +108,12 @@ pub(super) fn explicit_item_bounds(
..
}) => {
let substs = InternalSubsts::identity_for_item(tcx, def_id);
let item_ty = if *in_trait || rpitit_info.is_some() {
let item_ty = if *in_trait && !tcx.lower_impl_trait_in_trait_to_assoc_ty() {
tcx.mk_projection(def_id, substs)
} else {
tcx.mk_opaque(def_id, substs)
};
opaque_type_bounds(tcx, bounds_def_id, bounds, item_ty, *span)
opaque_type_bounds(tcx, def_id, bounds, item_ty, *span)
}
_ => bug!("item_bounds called on {:?}", def_id),
}
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_hir_analysis/src/variance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,14 @@ fn variance_of_opaque(tcx: TyCtxt<'_>, item_def_id: LocalDefId) -> &[ty::Varianc
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
match t.kind() {
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if matches!(
self.tcx.def_kind(*def_id),
DefKind::OpaqueTy | DefKind::ImplTraitPlaceholder
) =>
if matches!(self.tcx.def_kind(*def_id), DefKind::OpaqueTy) =>
{
self.visit_opaque(*def_id, substs)
}
// FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty) check whether this is necessary
// at all for RPITITs.
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if self.tcx.is_impl_trait_in_trait(*def_id) =>
{
self.visit_opaque(*def_id, substs)
}
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use super::{check_fn, Expectation, FnCtxt, GeneratorTypes};

use hir::def::DefKind;
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_hir::lang_items::LangItem;
Expand Down Expand Up @@ -713,14 +712,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.subst_iter_copied(self.tcx, substs)
.find_map(|(p, s)| get_future_output(p, s))?,
ty::Error(_) => return None,
ty::Alias(ty::Projection, proj)
if self.tcx.def_kind(proj.def_id) == DefKind::ImplTraitPlaceholder =>
{
self.tcx
.bound_explicit_item_bounds(proj.def_id)
.subst_iter_copied(self.tcx, proj.substs)
.find_map(|(p, s)| get_future_output(p, s))?
}
ty::Alias(ty::Projection, proj) if self.tcx.is_impl_trait_in_trait(proj.def_id) => self
.tcx
.bound_explicit_item_bounds(proj.def_id)
.subst_iter_copied(self.tcx, proj.substs)
.find_map(|(p, s)| get_future_output(p, s))?,
_ => span_bug!(
self.tcx.def_span(expr_def_id),
"async fn generator return type not an inference variable: {ret_ty}"
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,12 @@ impl<'tcx> InferCtxt<'tcx> {
pub fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
let (def_id, substs) = match *ty.kind() {
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if matches!(
self.tcx.def_kind(def_id),
DefKind::OpaqueTy | DefKind::ImplTraitPlaceholder
) =>
if matches!(self.tcx.def_kind(def_id), DefKind::OpaqueTy) =>
{
(def_id, substs)
}
ty::Alias(_, ty::AliasTy { def_id, substs, .. })
if self.tcx.is_impl_trait_in_trait(def_id) =>
{
(def_id, substs)
}
Expand Down Expand Up @@ -1754,8 +1756,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
)
}
(true, ty::Alias(ty::Projection, proj))
if self.tcx.def_kind(proj.def_id)
== DefKind::ImplTraitPlaceholder =>
if self.tcx.is_impl_trait_in_trait(proj.def_id) =>
{
let sm = self.tcx.sess.source_map();
let pos = sm.lookup_char_pos(self.tcx.def_span(proj.def_id).lo());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::TypeErrCtxt;
use rustc_errors::Applicability::{MachineApplicable, MaybeIncorrect};
use rustc_errors::{pluralize, Diagnostic, MultiSpan};
use rustc_hir::{self as hir, def::DefKind};
use rustc_hir as hir;
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::error::ExpectedFound;
use rustc_middle::ty::print::Printer;
Expand Down Expand Up @@ -75,7 +75,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
diag.note("an associated type was expected, but a different one was found");
}
(ty::Param(p), ty::Alias(ty::Projection, proj)) | (ty::Alias(ty::Projection, proj), ty::Param(p))
if tcx.def_kind(proj.def_id) != DefKind::ImplTraitPlaceholder =>
if !tcx.is_impl_trait_in_trait(proj.def_id) =>
{
let p_def_id = tcx
.generics_of(body_owner_def_id)
Expand Down Expand Up @@ -222,7 +222,7 @@ impl<T> Trait<T> for X {
diag.span_label(p_span, "this type parameter");
}
}
(ty::Alias(ty::Projection, proj_ty), _) if tcx.def_kind(proj_ty.def_id) != DefKind::ImplTraitPlaceholder => {
(ty::Alias(ty::Projection, proj_ty), _) if !tcx.is_impl_trait_in_trait(proj_ty.def_id) => {
self.expected_projection(
diag,
proj_ty,
Expand All @@ -231,7 +231,7 @@ impl<T> Trait<T> for X {
cause.code(),
);
}
(_, ty::Alias(ty::Projection, proj_ty)) if tcx.def_kind(proj_ty.def_id) != DefKind::ImplTraitPlaceholder => {
(_, ty::Alias(ty::Projection, 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
8 changes: 2 additions & 6 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::errors::OpaqueHiddenTypeDiag;
use crate::infer::{DefiningAnchor, InferCtxt, InferOk};
use crate::traits;
use hir::def::DefKind;
use hir::def_id::{DefId, LocalDefId};
use hir::OpaqueTyOrigin;
use rustc_data_structures::sync::Lrc;
Expand Down Expand Up @@ -481,9 +480,7 @@ where
}
}

ty::Alias(ty::Projection, proj)
if self.tcx.def_kind(proj.def_id) == DefKind::ImplTraitPlaceholder =>
{
ty::Alias(ty::Projection, proj) if self.tcx.is_impl_trait_in_trait(proj.def_id) => {
// Skip lifetime parameters that are not captures.
let variances = self.tcx.variances_of(proj.def_id);

Expand Down Expand Up @@ -563,8 +560,7 @@ impl<'tcx> InferCtxt<'tcx> {
// FIXME(RPITIT): Don't replace RPITITs with inference vars.
ty::Alias(ty::Projection, projection_ty)
if !projection_ty.has_escaping_bound_vars()
&& tcx.def_kind(projection_ty.def_id)
!= DefKind::ImplTraitPlaceholder =>
&& !tcx.is_impl_trait_in_trait(projection_ty.def_id) =>
{
self.infer_projection(
param_env,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ fn should_encode_type(tcx: TyCtxt<'_>, def_id: LocalDefId, def_kind: DefKind) ->
| DefKind::InlineConst => true,

DefKind::ImplTraitPlaceholder => {
let parent_def_id = tcx.impl_trait_in_trait_parent(def_id.to_def_id());
let parent_def_id = tcx.impl_trait_in_trait_parent_fn(def_id.to_def_id());
let assoc_item = tcx.associated_item(parent_def_id);
match assoc_item.container {
// Always encode an RPIT in an impl fn, since it always has a body
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ rustc_queries! {
{
desc { "determine whether the opaque is a type-alias impl trait" }
separate_provide_extern
feedable
}

query unsizing_params_for_adt(key: DefId) -> &'tcx rustc_index::bit_set::BitSet<u32>
Expand Down
20 changes: 15 additions & 5 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2552,12 +2552,18 @@ impl<'tcx> TyCtxt<'tcx> {
matches!(self.trait_of_item(def_id), Some(trait_id) if self.has_attr(trait_id, sym::const_trait))
}

pub fn impl_trait_in_trait_parent(self, mut def_id: DefId) -> DefId {
while let def_kind = self.def_kind(def_id) && def_kind != DefKind::AssocFn {
debug_assert_eq!(def_kind, DefKind::ImplTraitPlaceholder);
def_id = self.parent(def_id);
pub fn impl_trait_in_trait_parent_fn(self, mut def_id: DefId) -> DefId {
match self.opt_rpitit_info(def_id) {
Some(ImplTraitInTraitData::Trait { fn_def_id, .. })
| Some(ImplTraitInTraitData::Impl { fn_def_id, .. }) => fn_def_id,
None => {
while let def_kind = self.def_kind(def_id) && def_kind != DefKind::AssocFn {
debug_assert_eq!(def_kind, DefKind::ImplTraitPlaceholder);
def_id = self.parent(def_id);
}
def_id
}
}
def_id
}

pub fn impl_method_has_trait_impl_trait_tys(self, def_id: DefId) -> bool {
Expand All @@ -2572,6 +2578,10 @@ impl<'tcx> TyCtxt<'tcx> {

let Some(trait_item_def_id) = item.trait_item_def_id else { return false; };

if self.lower_impl_trait_in_trait_to_assoc_ty() {
return !self.associated_items_for_impl_trait_in_trait(trait_item_def_id).is_empty();
}

// FIXME(RPITIT): This does a somewhat manual walk through the signature
// of the trait fn to look for any RPITITs, but that's kinda doing a lot
// of work. We can probably remove this when we refactor RPITITs to be
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ pub trait PrettyPrinter<'tcx>:
}
ty::Alias(ty::Projection, ref data) => {
if !(self.should_print_verbose() || NO_QUERIES.with(|q| q.get()))
&& self.tcx().def_kind(data.def_id) == DefKind::ImplTraitPlaceholder
&& self.tcx().is_impl_trait_in_trait(data.def_id)
{
return self.pretty_print_opaque_impl_type(data.def_id, data.substs);
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ impl<'tcx> AliasTy<'tcx> {
match tcx.def_kind(self.def_id) {
DefKind::AssocTy | DefKind::AssocConst => tcx.parent(self.def_id),
DefKind::ImplTraitPlaceholder => {
tcx.parent(tcx.impl_trait_in_trait_parent(self.def_id))
tcx.parent(tcx.impl_trait_in_trait_parent_fn(self.def_id))
}
kind => bug!("expected a projection AliasTy; found {kind:?}"),
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ where
projection.trait_ref_and_own_substs(tcx)
} else {
// HACK(RPITIT): Remove this when RPITITs are lowered to regular assoc tys
let def_id = tcx.impl_trait_in_trait_parent(projection.def_id);
let def_id = tcx.impl_trait_in_trait_parent_fn(projection.def_id);
let trait_generics = tcx.generics_of(def_id);
(
tcx.mk_trait_ref(def_id, projection.substs.truncate_to(tcx, trait_generics)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,22 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref: ty::PolyTraitRef<'tcx>,
obligation: &PredicateObligation<'tcx>,
) -> OnUnimplementedNote {
if self.tcx.opt_rpitit_info(obligation.cause.body_id.to_def_id()).is_some() {
return OnUnimplementedNote::default();
}

let (def_id, substs) = self
.impl_similar_to(trait_ref, obligation)
.unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().substs));
let trait_ref = trait_ref.skip_binder();

let body_hir = self.tcx.hir().local_def_id_to_hir_id(obligation.cause.body_id);
let mut flags =
vec![(sym::ItemContext, self.describe_enclosure(body_hir).map(|s| s.to_owned()))];
let mut flags = vec![];
// FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty): HIR is not present for RPITITs,
// but I guess we could synthesize one here. We don't see any errors that rely on
// that yet, though.
let enclosure =
if let Some(body_hir) = self.tcx.opt_local_def_id_to_hir_id(obligation.cause.body_id) {
self.describe_enclosure(body_hir).map(|s| s.to_owned())
} else {
None
};
flags.push((sym::ItemContext, enclosure));

match obligation.cause.code() {
ObligationCauseCode::BuiltinDerivedObligation(..)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,7 @@ fn assemble_candidate_for_impl_trait_in_trait<'cx, 'tcx>(
) {
let tcx = selcx.tcx();
if tcx.def_kind(obligation.predicate.def_id) == DefKind::ImplTraitPlaceholder {
let trait_fn_def_id = tcx.impl_trait_in_trait_parent(obligation.predicate.def_id);
let trait_fn_def_id = tcx.impl_trait_in_trait_parent_fn(obligation.predicate.def_id);

let trait_def_id = tcx.parent(trait_fn_def_id);
let trait_substs =
Expand Down Expand Up @@ -2193,7 +2193,7 @@ fn confirm_impl_trait_in_trait_candidate<'tcx>(
let tcx = selcx.tcx();
let mut obligations = data.nested;

let trait_fn_def_id = tcx.impl_trait_in_trait_parent(obligation.predicate.def_id);
let trait_fn_def_id = tcx.impl_trait_in_trait_parent_fn(obligation.predicate.def_id);
let leaf_def = match specialization_graph::assoc_def(tcx, data.impl_def_id, trait_fn_def_id) {
Ok(assoc_ty) => assoc_ty,
Err(guar) => return Progress::error(tcx, guar),
Expand Down
Loading