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

Make RPITITs inherit the assumed_wf_types of their parent method #113704

Merged
merged 3 commits into from
Jul 30, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 11 additions & 88 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,17 @@ fn check_trait_item(tcx: TyCtxt<'_>, trait_item: &hir::TraitItem<'_>) {
};
check_object_unsafe_self_trait_by_name(tcx, trait_item);
check_associated_item(tcx, def_id, span, method_sig);

if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) {
for &assoc_ty_def_id in tcx.associated_types_for_impl_traits_in_associated_fn(def_id) {
check_associated_item(
tcx,
assoc_ty_def_id.expect_local(),
tcx.def_span(assoc_ty_def_id),
None,
);
}
}
}

/// Require that the user writes where clauses on GATs for the implicit
Expand Down Expand Up @@ -1466,13 +1477,6 @@ fn check_fn_or_method<'tcx>(

check_where_clauses(wfcx, span, def_id);

check_return_position_impl_trait_in_trait_bounds(
wfcx,
def_id,
sig.output(),
hir_decl.output.span(),
);

if sig.abi == Abi::RustCall {
let span = tcx.def_span(def_id);
let has_implicit_self = hir_decl.implicit_self != hir::ImplicitSelfKind::None;
Expand Down Expand Up @@ -1507,87 +1511,6 @@ fn check_fn_or_method<'tcx>(
}
}

/// Basically `check_associated_type_bounds`, but separated for now and should be
/// deduplicated when RPITITs get lowered into real associated items.
#[tracing::instrument(level = "trace", skip(wfcx))]
fn check_return_position_impl_trait_in_trait_bounds<'tcx>(
wfcx: &WfCheckingCtxt<'_, 'tcx>,
fn_def_id: LocalDefId,
fn_output: Ty<'tcx>,
span: Span,
) {
let tcx = wfcx.tcx();
let Some(assoc_item) = tcx.opt_associated_item(fn_def_id.to_def_id()) else {
return;
};
if assoc_item.container != ty::AssocItemContainer::TraitContainer {
return;
}
fn_output.visit_with(&mut ImplTraitInTraitFinder {
wfcx,
fn_def_id,
depth: ty::INNERMOST,
seen: FxHashSet::default(),
});
}

// 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 args 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.
struct ImplTraitInTraitFinder<'a, 'tcx> {
wfcx: &'a WfCheckingCtxt<'a, 'tcx>,
fn_def_id: LocalDefId,
depth: ty::DebruijnIndex,
seen: FxHashSet<DefId>,
}
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ImplTraitInTraitFinder<'_, 'tcx> {
type BreakTy = !;

fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<!> {
let tcx = self.wfcx.tcx();
if let ty::Alias(ty::Opaque, unshifted_opaque_ty) = *ty.kind()
&& self.seen.insert(unshifted_opaque_ty.def_id)
&& let Some(opaque_def_id) = unshifted_opaque_ty.def_id.as_local()
&& let origin = tcx.opaque_type_origin(opaque_def_id)
&& let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = origin
&& source == self.fn_def_id
{
let opaque_ty = tcx.fold_regions(unshifted_opaque_ty, |re, _depth| {
match re.kind() {
ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReError(_) | ty::ReStatic => re,
r => bug!("unexpected region: {r:?}"),
}
});
for (bound, bound_span) in tcx
.explicit_item_bounds(opaque_ty.def_id)
.iter_instantiated_copied(tcx, opaque_ty.args)
{
let bound = self.wfcx.normalize(bound_span, None, bound);
self.wfcx.register_obligations(traits::wf::predicate_obligations(
self.wfcx.infcx,
self.wfcx.param_env,
self.wfcx.body_def_id,
bound.as_predicate(),
bound_span,
));
// Set the debruijn index back to innermost here, since we already eagerly
// shifted the args that we use to generate these bounds. This is unfortunately
// subtly different behavior than the `ImplTraitInTraitFinder` we use in `param_env`,
// but that function doesn't actually need to normalize the bound it's visiting
// (whereas we have to do so here)...
let old_depth = std::mem::replace(&mut self.depth, ty::INNERMOST);
bound.visit_with(self);
self.depth = old_depth;
}
}
ty.super_visit_with(self)
}
}

const HELP_FOR_SELF_TYPE: &str = "consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
`self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
of the previous types except `Self`)";
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ provide! { tcx, def_id, other, cdata,
debug_assert_eq!(tcx.def_kind(def_id), DefKind::OpaqueTy);
cdata.root.tables.is_type_alias_impl_trait.get(cdata, def_id.index)
}
assumed_wf_types_for_rpitit => { table }
collect_return_position_impl_trait_in_trait_tys => {
Ok(cdata
.root
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,12 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
if let Some(rpitit_info) = item.opt_rpitit_info {
record!(self.tables.opt_rpitit_info[def_id] <- rpitit_info);
if matches!(rpitit_info, ty::ImplTraitInTraitData::Trait { .. }) {
record_array!(
self.tables.assumed_wf_types_for_rpitit[def_id]
<- self.tcx.assumed_wf_types_for_rpitit(def_id)
);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ define_tables! {
trait_impl_trait_tys: Table<DefIndex, LazyValue<FxHashMap<DefId, ty::EarlyBinder<Ty<'static>>>>>,
doc_link_resolutions: Table<DefIndex, LazyValue<DocLinkResMap>>,
doc_link_traits_in_scope: Table<DefIndex, LazyArray<DefId>>,
assumed_wf_types_for_rpitit: Table<DefIndex, LazyArray<(Ty<'static>, Span)>>,
}

#[derive(TyEncodable, TyDecodable)]
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,13 @@ rustc_queries! {
desc { |tcx| "computing the implied bounds of `{}`", tcx.def_path_str(key) }
}

/// We need to store the assumed_wf_types for an RPITIT so that impls of foreign
/// traits with return-position impl trait in traits can inherit the right wf types.
query assumed_wf_types_for_rpitit(key: DefId) -> &'tcx [(Ty<'tcx>, Span)] {
desc { |tcx| "computing the implied bounds of `{}`", tcx.def_path_str(key) }
separate_provide_extern
}

/// Computes the signature of the function.
query fn_sig(key: DefId) -> ty::EarlyBinder<ty::PolyFnSig<'tcx>> {
desc { |tcx| "computing function signature of `{}`", tcx.def_path_str(key) }
Expand Down
86 changes: 85 additions & 1 deletion compiler/rustc_ty_utils/src/implied_bounds.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::middle::resolve_bound_vars as rbv;
use rustc_middle::query::Providers;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::Span;
use std::iter;

pub fn provide(providers: &mut Providers) {
*providers = Providers { assumed_wf_types, ..*providers };
*providers = Providers {
assumed_wf_types,
assumed_wf_types_for_rpitit: |tcx, def_id| {
assert!(tcx.is_impl_trait_in_trait(def_id.to_def_id()));
tcx.assumed_wf_types(def_id)
},
..*providers
};
}

fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &'tcx [(Ty<'tcx>, Span)] {
Expand Down Expand Up @@ -42,6 +51,81 @@ fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &'tcx [(Ty<'
let mut impl_spans = impl_spans(tcx, def_id);
tcx.arena.alloc_from_iter(tys.into_iter().map(|ty| (ty, impl_spans.next().unwrap())))
}
DefKind::AssocTy if let Some(data) = tcx.opt_rpitit_info(def_id.to_def_id()) => match data {
ty::ImplTraitInTraitData::Trait { fn_def_id, opaque_def_id } => {
let hir::OpaqueTy { lifetime_mapping, .. } =
*tcx.hir().expect_item(opaque_def_id.expect_local()).expect_opaque_ty();
// We need to remap all of the late-bound lifetimes in theassumed wf types
// of the fn (which are represented as ReFree) to the early-bound lifetimes
// of the RPITIT (which are represented by ReEarlyBound owned by the opaque).
// Luckily, this is very easy to do because we already have that mapping
// stored in the HIR of this RPITIT.
//
// Side-note: We don't really need to do this remapping for early-bound
// lifetimes because they're already "linked" by the bidirectional outlives
// predicates we insert in the `explicit_predicates_of` query for RPITITs.
let mut mapping = FxHashMap::default();
let generics = tcx.generics_of(def_id);
for &(lifetime, new_early_bound_def_id) in
lifetime_mapping.expect("expected lifetime mapping for RPITIT")
{
if let Some(rbv::ResolvedArg::LateBound(_, _, def_id)) =
tcx.named_bound_var(lifetime.hir_id)
{
let name = tcx.hir().name(lifetime.hir_id);
let index = generics
.param_def_id_to_index(tcx, new_early_bound_def_id.to_def_id())
.unwrap();
mapping.insert(
ty::Region::new_free(
tcx,
fn_def_id,
ty::BoundRegionKind::BrNamed(def_id, name),
),
ty::Region::new_early_bound(
tcx,
ty::EarlyBoundRegion {
def_id: new_early_bound_def_id.to_def_id(),
index,
name,
},
),
);
}
}
// FIXME: This could use a real folder, I guess.
let remapped_wf_tys = tcx.fold_regions(
tcx.assumed_wf_types(fn_def_id.expect_local()).to_vec(),
|region, _| {
// If `region` is a `ReFree` that is captured by the
// opaque, remap it to its corresponding the early-
// bound region.
if let Some(remapped_region) = mapping.get(&region) {
*remapped_region
} else {
region
}
},
);
tcx.arena.alloc_from_iter(remapped_wf_tys)
}
// Assumed wf types for RPITITs in an impl just inherit (and instantiate)
// the assumed wf types of the trait's RPITIT GAT.
ty::ImplTraitInTraitData::Impl { .. } => {
let impl_def_id = tcx.local_parent(def_id);
let rpitit_def_id = tcx.associated_item(def_id).trait_item_def_id.unwrap();
let args = ty::GenericArgs::identity_for_item(tcx, def_id).rebase_onto(
tcx,
impl_def_id.to_def_id(),
tcx.impl_trait_ref(impl_def_id).unwrap().instantiate_identity().args,
);
tcx.arena.alloc_from_iter(
ty::EarlyBinder::bind(tcx.assumed_wf_types_for_rpitit(rpitit_def_id))
.iter_instantiated_copied(tcx, args)
.chain(tcx.assumed_wf_types(impl_def_id).into_iter().copied()),
)
}
},
DefKind::AssocConst | DefKind::AssocTy => tcx.assumed_wf_types(tcx.local_parent(def_id)),
DefKind::OpaqueTy => match tcx.def_kind(tcx.local_parent(def_id)) {
DefKind::TyAlias => ty::List::empty(),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ty_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#![feature(assert_matches)]
#![feature(iterator_try_collect)]
#![feature(let_chains)]
#![feature(if_let_guard)]
#![feature(never_type)]
#![feature(box_patterns)]
#![recursion_limit = "256"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error[E0311]: the parameter type `U` may not live long enough
LL | async fn foo(&self) -> &(T, U) where T: Debug + Sized, U: Hash;
| ^^^^^^^
|
note: the parameter type `U` must be valid for the anonymous lifetime defined here...
note: the parameter type `U` must be valid for the anonymous lifetime as defined here...
--> $DIR/async-generics-and-bounds.rs:12:18
|
LL | async fn foo(&self) -> &(T, U) where T: Debug + Sized, U: Hash;
| ^^^^^
| ^
note: ...so that the reference type `&(T, U)` does not outlive the data it points at
--> $DIR/async-generics-and-bounds.rs:12:28
|
Expand All @@ -21,11 +21,11 @@ error[E0311]: the parameter type `T` may not live long enough
LL | async fn foo(&self) -> &(T, U) where T: Debug + Sized, U: Hash;
| ^^^^^^^
|
note: the parameter type `T` must be valid for the anonymous lifetime defined here...
note: the parameter type `T` must be valid for the anonymous lifetime as defined here...
--> $DIR/async-generics-and-bounds.rs:12:18
|
LL | async fn foo(&self) -> &(T, U) where T: Debug + Sized, U: Hash;
| ^^^^^
| ^
note: ...so that the reference type `&(T, U)` does not outlive the data it points at
--> $DIR/async-generics-and-bounds.rs:12:28
|
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/async-await/in-trait/async-generics.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error[E0311]: the parameter type `U` may not live long enough
LL | async fn foo(&self) -> &(T, U);
| ^^^^^^^
|
note: the parameter type `U` must be valid for the anonymous lifetime defined here...
note: the parameter type `U` must be valid for the anonymous lifetime as defined here...
--> $DIR/async-generics.rs:9:18
|
LL | async fn foo(&self) -> &(T, U);
| ^^^^^
| ^
note: ...so that the reference type `&(T, U)` does not outlive the data it points at
--> $DIR/async-generics.rs:9:28
|
Expand All @@ -21,11 +21,11 @@ error[E0311]: the parameter type `T` may not live long enough
LL | async fn foo(&self) -> &(T, U);
| ^^^^^^^
|
note: the parameter type `T` must be valid for the anonymous lifetime defined here...
note: the parameter type `T` must be valid for the anonymous lifetime as defined here...
--> $DIR/async-generics.rs:9:18
|
LL | async fn foo(&self) -> &(T, U);
| ^^^^^
| ^
note: ...so that the reference type `&(T, U)` does not outlive the data it points at
--> $DIR/async-generics.rs:9:28
|
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/impl-trait/in-trait/assumed-wf-bounds-in-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// check-pass
// edition: 2021
// issue: 113796

#![feature(async_fn_in_trait)]

trait AsyncLendingIterator {
type Item<'a>
where
Self: 'a;

async fn next(&mut self) -> Option<Self::Item<'_>>;
}

struct Lend<I>(I);
impl<I> AsyncLendingIterator for Lend<I> {
type Item<'a> = &'a I
where
Self: 'a;

// Checking that the synthetic `<Self as AsyncLendingIterator>::next()` GAT
// is well-formed requires being able to assume the WF types of `next`.

async fn next(&mut self) -> Option<Self::Item<'_>> {
todo!()
}
}

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ impl<'a, I: 'a + Iterable> Iterable for &'a I {
//~^ ERROR impl has stricter requirements than trait

fn iter(&self) -> impl 'a + Iterator<Item = I::Item<'a>> {
//~^ ERROR the type `&'a I` does not fulfill the required lifetime
(*self).iter()
}
}
Expand Down
17 changes: 2 additions & 15 deletions tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,6 @@ help: copy the `where` clause predicates from the trait
LL | where Self: 'b;
| ~~~~~~~~~~~~~~

error[E0477]: the type `&'a I` does not fulfill the required lifetime
--> $DIR/bad-item-bound-within-rpitit.rs:19:23
|
LL | fn iter(&self) -> impl 'a + Iterator<Item = I::Item<'a>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: type must outlive the anonymous lifetime as defined here
--> $DIR/bad-item-bound-within-rpitit.rs:10:28
|
LL | fn iter(&self) -> impl '_ + Iterator<Item = Self::Item<'_>>;
| ^^

error: aborting due to 2 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0276, E0477.
For more information about an error, try `rustc --explain E0276`.
For more information about this error, try `rustc --explain E0276`.