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

Give return-position impl traits in trait a (synthetic) name to avoid name collisions with new lowering strategy #109455

Closed
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
22 changes: 12 additions & 10 deletions compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ pub enum DefPathData {
/// An `impl Trait` type node.
ImplTrait,
/// `impl Trait` generated associated type node.
ImplTraitAssocTy,
ImplTraitAssocTy(Symbol),
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess there's no real reason why this needs to be a separate def path data. We could use TypeNs here now, (I guess) as long as we keep the actual name that the RPITIT is lowering to unnameable.

}

impl Definitions {
Expand Down Expand Up @@ -402,11 +402,11 @@ impl DefPathData {
pub fn get_opt_name(&self) -> Option<Symbol> {
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,
Expand All @@ -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 },
Expand All @@ -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 },
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_symbol_mangling/src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
81 changes: 54 additions & 27 deletions compiler/rustc_ty_utils/src/assoc.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
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 {
Expand Down Expand Up @@ -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<LocalDefId>,
struct RPITVisitor<'tcx> {
rpits: FxIndexSet<LocalDefId>,
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);
Expand All @@ -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()
},
),
)
Expand All @@ -254,13 +257,17 @@ 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 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();
Expand All @@ -275,14 +282,14 @@ 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,
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(),
}),
});
Expand Down Expand Up @@ -324,7 +331,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,
Expand All @@ -335,7 +342,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: &[],
});

Expand All @@ -345,22 +352,42 @@ 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
/// that the `impl Trait` comes from, synthesize an associated type for that `impl 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);
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();
Expand All @@ -375,10 +402,10 @@ 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.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() }),
Expand Down
9 changes: 6 additions & 3 deletions tests/ui/impl-trait/in-trait/auxiliary/rpitit.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// [next] compile-flags: -Zlower-impl-trait-in-trait-to-assoc-ty

#![feature(return_position_impl_trait_in_trait)]

use std::ops::Deref;

pub trait Foo {
fn bar() -> impl Sized;
fn bar() -> impl Deref<Target = impl Sized>;
}

pub struct Foreign;

impl Foo for Foreign {
fn bar() {}
fn bar() -> &'static () { &() }
}
4 changes: 2 additions & 2 deletions tests/ui/impl-trait/in-trait/doesnt-satisfy.next.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 12 additions & 2 deletions tests/ui/impl-trait/in-trait/foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> { Arc::new(String::new()) }
}

fn main() {
// Witness an RPITIT from another crate
let () = <rpitit::Foreign as rpitit::Foo>::bar();
// Witness an RPITIT from another crate.
let &() = <rpitit::Foreign as rpitit::Foo>::bar();

let x: Arc<String> = <Local as rpitit::Foo>::bar();
}