Skip to content

Commit

Permalink
Rollup merge of #109198 - compiler-errors:new-rpitit-default-body, r=…
Browse files Browse the repository at this point in the history
…spastorino

Install projection from RPITIT to default trait method opaque correctly

1. For new lowering strategy `-Zlower-impl-trait-in-trait-to-assoc-ty`, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass!

2. Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in `check_return_position_impl_trait_in_trait_bounds`. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why.

3. Also, just a small drive-by for `rustc_on_unimplemented`. Not sure if it affects any tests, but can't hurt.

r? ````@spastorino,```` based off of #109140
  • Loading branch information
matthiaskrgr authored Mar 17, 2023
2 parents edd6b42 + 8d922eb commit 246d989
Show file tree
Hide file tree
Showing 50 changed files with 419 additions and 107 deletions.
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 @@ -1548,16 +1547,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
{
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 @@ -715,14 +714,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 @@ -1757,8 +1759,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
Expand Up @@ -3,7 +3,6 @@ use super::{DefineOpaqueTypes, InferResult};
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 @@ -478,9 +477,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 @@ -559,8 +556,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 @@ -1298,7 +1298,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 @@ -2200,7 +2200,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

0 comments on commit 246d989

Please sign in to comment.