From 75a9f5d92c0b4883896a4ac4aca228a46fc3690f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 20 Mar 2023 13:20:28 -0300 Subject: [PATCH 1/3] Always encode RPITITs --- compiler/rustc_metadata/src/rmeta/encoder.rs | 8 +++++++- tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index ef3eda584e1b8..44a57770221a2 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1044,7 +1044,13 @@ fn should_encode_type(tcx: TyCtxt<'_>, def_id: LocalDefId, def_kind: DefKind) -> let assoc_item = tcx.associated_item(def_id); match assoc_item.container { ty::AssocItemContainer::ImplContainer => true, - ty::AssocItemContainer::TraitContainer => assoc_item.defaultness(tcx).has_value(), + // FIXME(-Zlower-impl-trait-in-trait-to-assoc-ty) always encode RPITITs, + // since we need to be able to "project" from an RPITIT associated item + // to an opaque when installing the default projection predicates in + // default trait methods with RPITITs. + ty::AssocItemContainer::TraitContainer => { + assoc_item.defaultness(tcx).has_value() || assoc_item.opt_rpitit_info.is_some() + } } } DefKind::TyParam => { diff --git a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs index 74df300f85a6b..74f7bc603aafc 100644 --- a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs +++ b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs @@ -1,3 +1,5 @@ +// [next] compile-flags: -Zlower-impl-trait-in-trait-to-assoc-ty + #![feature(return_position_impl_trait_in_trait)] pub trait Foo { From f8aea8d60c90a53f11b53f92a779861512011a38 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 21 Mar 2023 18:06:04 +0000 Subject: [PATCH 2/3] Walk return-position impl trait in trait deeply in associated_item_def_ids --- compiler/rustc_ty_utils/src/assoc.rs | 52 +++++++++++-------- .../impl-trait/in-trait/auxiliary/rpitit.rs | 7 +-- tests/ui/impl-trait/in-trait/foreign.rs | 14 ++++- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs index 6d9ad96fa74d4..012c0008b0025 100644 --- a/compiler/rustc_ty_utils/src/assoc.rs +++ b/compiler/rustc_ty_utils/src/assoc.rs @@ -1,3 +1,4 @@ +use rustc_data_structures::fx::FxIndexSet; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId}; @@ -196,20 +197,26 @@ fn associated_types_for_impl_traits_in_associated_fn( match tcx.def_kind(parent_def_id) { DefKind::Trait => { - struct RPITVisitor { - rpits: Vec, + struct RPITVisitor<'tcx> { + rpits: FxIndexSet, + tcx: TyCtxt<'tcx>, } - impl<'v> Visitor<'v> for RPITVisitor { - fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) { - if let hir::TyKind::OpaqueDef(item_id, _, _) = ty.kind { - self.rpits.push(item_id.owner_id.def_id) + impl<'tcx> Visitor<'tcx> for RPITVisitor<'tcx> { + fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) { + if let hir::TyKind::OpaqueDef(item_id, _, _) = ty.kind + && self.rpits.insert(item_id.owner_id.def_id) + { + let opaque_item = self.tcx.hir().expect_item(item_id.owner_id.def_id).expect_opaque_ty(); + for bound in opaque_item.bounds { + intravisit::walk_param_bound(self, bound); + } } intravisit::walk_ty(self, ty) } } - let mut visitor = RPITVisitor { rpits: Vec::new() }; + let mut visitor = RPITVisitor { tcx, rpits: FxIndexSet::default() }; if let Some(output) = tcx.hir().get_fn_output(fn_def_id) { visitor.visit_fn_ret_ty(output); @@ -227,13 +234,9 @@ fn associated_types_for_impl_traits_in_associated_fn( tcx.arena.alloc_from_iter( tcx.associated_types_for_impl_traits_in_associated_fn(trait_fn_def_id).iter().map( - move |trait_assoc_def_id| { - associated_type_for_impl_trait_in_impl( - tcx, - trait_assoc_def_id.expect_local(), - fn_def_id, - ) - .to_def_id() + move |&trait_assoc_def_id| { + associated_type_for_impl_trait_in_impl(tcx, trait_assoc_def_id, fn_def_id) + .to_def_id() }, ), ) @@ -254,13 +257,16 @@ fn associated_type_for_impl_trait_in_trait( tcx: TyCtxt<'_>, opaque_ty_def_id: LocalDefId, ) -> LocalDefId { - let fn_def_id = tcx.impl_trait_in_trait_parent_fn(opaque_ty_def_id.to_def_id()); - let trait_def_id = tcx.parent(fn_def_id); + let (hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id)) = + tcx.hir().expect_item(opaque_ty_def_id).expect_opaque_ty().origin + else { + bug!("expected opaque for {opaque_ty_def_id:?}"); + }; + let trait_def_id = tcx.local_parent(fn_def_id); assert_eq!(tcx.def_kind(trait_def_id), DefKind::Trait); let span = tcx.def_span(opaque_ty_def_id); - let trait_assoc_ty = - tcx.at(span).create_def(trait_def_id.expect_local(), DefPathData::ImplTraitAssocTy); + let trait_assoc_ty = tcx.at(span).create_def(trait_def_id, DefPathData::ImplTraitAssocTy); let local_def_id = trait_assoc_ty.def_id(); let def_id = local_def_id.to_def_id(); @@ -282,7 +288,7 @@ fn associated_type_for_impl_trait_in_trait( container: ty::TraitContainer, fn_has_self_parameter: false, opt_rpitit_info: Some(ImplTraitInTraitData::Trait { - fn_def_id, + fn_def_id: fn_def_id.to_def_id(), opaque_def_id: opaque_ty_def_id.to_def_id(), }), }); @@ -324,7 +330,7 @@ fn associated_type_for_impl_trait_in_trait( params.iter().map(|param| (param.def_id, param.index)).collect(); ty::Generics { - parent: Some(trait_def_id), + parent: Some(trait_def_id.to_def_id()), parent_count, params, param_def_id_to_index, @@ -335,7 +341,7 @@ fn associated_type_for_impl_trait_in_trait( // There are no predicates for the synthesized associated type. trait_assoc_ty.explicit_predicates_of(ty::GenericPredicates { - parent: Some(trait_def_id), + parent: Some(trait_def_id.to_def_id()), predicates: &[], }); @@ -352,7 +358,7 @@ fn associated_type_for_impl_trait_in_trait( /// that inherits properties that we infer from the method and the associated type. fn associated_type_for_impl_trait_in_impl( tcx: TyCtxt<'_>, - trait_assoc_def_id: LocalDefId, + trait_assoc_def_id: DefId, impl_fn_def_id: LocalDefId, ) -> LocalDefId { let impl_local_def_id = tcx.local_parent(impl_fn_def_id); @@ -378,7 +384,7 @@ fn associated_type_for_impl_trait_in_impl( name: kw::Empty, kind: ty::AssocKind::Type, def_id, - trait_item_def_id: Some(trait_assoc_def_id.to_def_id()), + trait_item_def_id: Some(trait_assoc_def_id), container: ty::ImplContainer, fn_has_self_parameter: false, opt_rpitit_info: Some(ImplTraitInTraitData::Impl { fn_def_id: impl_fn_def_id.to_def_id() }), diff --git a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs index 74f7bc603aafc..ffeabe5c2edab 100644 --- a/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs +++ b/tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs @@ -2,12 +2,13 @@ #![feature(return_position_impl_trait_in_trait)] +use std::ops::Deref; + pub trait Foo { - fn bar() -> impl Sized; + fn bar() -> impl Deref; } pub struct Foreign; - impl Foo for Foreign { - fn bar() {} + fn bar() -> &'static () { &() } } diff --git a/tests/ui/impl-trait/in-trait/foreign.rs b/tests/ui/impl-trait/in-trait/foreign.rs index df77372aabdfc..f4972d948b246 100644 --- a/tests/ui/impl-trait/in-trait/foreign.rs +++ b/tests/ui/impl-trait/in-trait/foreign.rs @@ -5,7 +5,17 @@ extern crate rpitit; +use std::sync::Arc; + +// Implement an RPITIT from another crate. +struct Local; +impl rpitit::Foo for Local { + fn bar() -> Arc { Arc::new(String::new()) } +} + fn main() { - // Witness an RPITIT from another crate - let () = ::bar(); + // Witness an RPITIT from another crate. + let &() = ::bar(); + + let x: Arc = ::bar(); } From 602cf24e46c7509591055c59a389351d678a857e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 21 Mar 2023 17:41:14 +0000 Subject: [PATCH 3/3] Give RPITITs real names with -Zlower-impl-trait-in-trait-to-assoc-ty --- compiler/rustc_hir/src/definitions.rs | 22 +++++++------ .../src/typeid/typeid_itanium_cxx_abi.rs | 2 +- compiler/rustc_symbol_mangling/src/v0.rs | 2 +- compiler/rustc_ty_utils/src/assoc.rs | 31 ++++++++++++++++--- .../in-trait/doesnt-satisfy.next.stderr | 4 +-- 5 files changed, 42 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index 3c4fc9cb530a4..2af7d26986d34 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/compiler/rustc_hir/src/definitions.rs @@ -281,7 +281,7 @@ pub enum DefPathData { /// An `impl Trait` type node. ImplTrait, /// `impl Trait` generated associated type node. - ImplTraitAssocTy, + ImplTraitAssocTy(Symbol), } impl Definitions { @@ -402,11 +402,11 @@ impl DefPathData { pub fn get_opt_name(&self) -> Option { use self::DefPathData::*; match *self { - TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => Some(name), - - // We use this name when collecting `ModChild`s. - // FIXME this could probably be removed with some refactoring to the name resolver. - ImplTraitAssocTy => Some(kw::Empty), + TypeNs(name) + | ValueNs(name) + | MacroNs(name) + | LifetimeNs(name) + | ImplTraitAssocTy(name) => Some(name), Impl | ForeignMod | CrateRoot | Use | GlobalAsm | ClosureExpr | Ctor | AnonConst | ImplTrait => None, @@ -416,9 +416,11 @@ impl DefPathData { pub fn name(&self) -> DefPathDataName { use self::DefPathData::*; match *self { - TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => { - DefPathDataName::Named(name) - } + TypeNs(name) + | ValueNs(name) + | MacroNs(name) + | LifetimeNs(name) + | ImplTraitAssocTy(name) => DefPathDataName::Named(name), // Note that this does not show up in user print-outs. CrateRoot => DefPathDataName::Anon { namespace: kw::Crate }, Impl => DefPathDataName::Anon { namespace: kw::Impl }, @@ -428,7 +430,7 @@ impl DefPathData { ClosureExpr => DefPathDataName::Anon { namespace: sym::closure }, Ctor => DefPathDataName::Anon { namespace: sym::constructor }, AnonConst => DefPathDataName::Anon { namespace: sym::constant }, - ImplTrait | ImplTraitAssocTy => DefPathDataName::Anon { namespace: sym::opaque }, + ImplTrait => DefPathDataName::Anon { namespace: sym::opaque }, } } } diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 1a679f32ca59b..e469201ff0151 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -396,7 +396,7 @@ fn encode_ty_name(tcx: TyCtxt<'_>, def_id: DefId) -> String { hir::definitions::DefPathData::CrateRoot | hir::definitions::DefPathData::Use | hir::definitions::DefPathData::GlobalAsm - | hir::definitions::DefPathData::ImplTraitAssocTy + | hir::definitions::DefPathData::ImplTraitAssocTy(_) | hir::definitions::DefPathData::MacroNs(..) | hir::definitions::DefPathData::LifetimeNs(..) => { bug!("encode_ty_name: unexpected `{:?}`", disambiguated_data.data); diff --git a/compiler/rustc_symbol_mangling/src/v0.rs b/compiler/rustc_symbol_mangling/src/v0.rs index 2f20d42139c8d..3832208b8fec4 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -792,7 +792,7 @@ impl<'tcx> Printer<'tcx> for &mut SymbolMangler<'tcx> { | DefPathData::Use | DefPathData::GlobalAsm | DefPathData::Impl - | DefPathData::ImplTraitAssocTy + | DefPathData::ImplTraitAssocTy(_) | DefPathData::MacroNs(_) | DefPathData::LifetimeNs(_) => { bug!("symbol_names: unexpected DefPathData: {:?}", disambiguated_data.data) diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs index 012c0008b0025..aa9ddc9a122a0 100644 --- a/compiler/rustc_ty_utils/src/assoc.rs +++ b/compiler/rustc_ty_utils/src/assoc.rs @@ -5,7 +5,7 @@ use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId}; use rustc_hir::definitions::DefPathData; use rustc_hir::intravisit::{self, Visitor}; use rustc_middle::ty::{self, ImplTraitInTraitData, InternalSubsts, TyCtxt}; -use rustc_span::symbol::kw; +use rustc_span::Symbol; pub fn provide(providers: &mut ty::query::Providers) { *providers = ty::query::Providers { @@ -266,7 +266,8 @@ fn associated_type_for_impl_trait_in_trait( assert_eq!(tcx.def_kind(trait_def_id), DefKind::Trait); let span = tcx.def_span(opaque_ty_def_id); - let trait_assoc_ty = tcx.at(span).create_def(trait_def_id, DefPathData::ImplTraitAssocTy); + let name = name_for_impl_trait_in_trait(tcx, opaque_ty_def_id, trait_def_id); + let trait_assoc_ty = tcx.at(span).create_def(trait_def_id, DefPathData::ImplTraitAssocTy(name)); let local_def_id = trait_assoc_ty.def_id(); let def_id = local_def_id.to_def_id(); @@ -281,7 +282,7 @@ fn associated_type_for_impl_trait_in_trait( trait_assoc_ty.def_ident_span(Some(span)); trait_assoc_ty.associated_item(ty::AssocItem { - name: kw::Empty, + name, kind: ty::AssocKind::Type, def_id, trait_item_def_id: None, @@ -351,6 +352,24 @@ fn associated_type_for_impl_trait_in_trait( local_def_id } +/// Create a stable path name for an associated type for an impl trait in trait +/// by appending the opaque type's path segments starting from the function name. +fn name_for_impl_trait_in_trait( + tcx: TyCtxt<'_>, + opaque_ty_def_id: LocalDefId, + trait_def_id: LocalDefId, +) -> Symbol { + let mut name = vec![]; + let mut def_id = opaque_ty_def_id; + while def_id != trait_def_id { + name.push(tcx.def_key(def_id.to_def_id()).disambiguated_data.to_string()); + def_id = tcx.local_parent(def_id); + } + name.reverse(); + let name = Symbol::intern(&name.join("::")); + name +} + /// Given an `trait_assoc_def_id` corresponding to an associated item synthesized /// from an `impl Trait` in an associated function from a trait, and an /// `impl_fn_def_id` that represents an implementation of the associated function @@ -364,9 +383,11 @@ fn associated_type_for_impl_trait_in_impl( let impl_local_def_id = tcx.local_parent(impl_fn_def_id); let impl_def_id = impl_local_def_id.to_def_id(); + let name = tcx.item_name(trait_assoc_def_id); // FIXME fix the span, we probably want the def_id of the return type of the function let span = tcx.def_span(impl_fn_def_id); - let impl_assoc_ty = tcx.at(span).create_def(impl_local_def_id, DefPathData::ImplTraitAssocTy); + let impl_assoc_ty = + tcx.at(span).create_def(impl_local_def_id, DefPathData::ImplTraitAssocTy(name)); let local_def_id = impl_assoc_ty.def_id(); let def_id = local_def_id.to_def_id(); @@ -381,7 +402,7 @@ fn associated_type_for_impl_trait_in_impl( impl_assoc_ty.def_ident_span(Some(span)); impl_assoc_ty.associated_item(ty::AssocItem { - name: kw::Empty, + name, kind: ty::AssocKind::Type, def_id, trait_item_def_id: Some(trait_assoc_def_id), diff --git a/tests/ui/impl-trait/in-trait/doesnt-satisfy.next.stderr b/tests/ui/impl-trait/in-trait/doesnt-satisfy.next.stderr index bbfa089ceef92..653016cf009a9 100644 --- a/tests/ui/impl-trait/in-trait/doesnt-satisfy.next.stderr +++ b/tests/ui/impl-trait/in-trait/doesnt-satisfy.next.stderr @@ -6,11 +6,11 @@ LL | fn bar() -> () {} | = help: the trait `std::fmt::Display` is not implemented for `()` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead -note: required by a bound in `Foo::{opaque#0}` +note: required by a bound in `Foo::bar::{opaque#0}` --> $DIR/doesnt-satisfy.rs:8:22 | LL | fn bar() -> impl std::fmt::Display; - | ^^^^^^^^^^^^^^^^^ required by this bound in `Foo::` + | ^^^^^^^^^^^^^^^^^ required by this bound in `Foo::bar::{opaque#0}` error: aborting due to previous error