Skip to content

Commit

Permalink
Make synthetic RPITIT assoc ty name handling more rigorous.
Browse files Browse the repository at this point in the history
Currently it relies on special treatment of `kw::Empty`, which is really
easy to get wrong. This commit makes the special case clearer in the
type system by using `Option`. It's a bit clumsy, but the synthetic name
handling itself is a bit clumsy; better to make it explicit than sneak
it in.

Fixes #133426.
  • Loading branch information
nnethercote committed Mar 4, 2025
1 parent 5dd8614 commit 292031d
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 36 deletions.
10 changes: 7 additions & 3 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl DefKind {
}

// Some `DefKind`s require a name, some don't. Panics if one is needed but
// not provided.
// not provided. (`AssocTy` is an exception, see below.)
pub fn def_path_data(self, name: Option<Symbol>) -> DefPathData {
match self {
DefKind::Mod
Expand All @@ -266,9 +266,13 @@ impl DefKind {
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::AssocTy
| DefKind::TyParam
| DefKind::ExternCrate => DefPathData::TypeNs(name.unwrap()),
| DefKind::ExternCrate => DefPathData::TypeNs(Some(name.unwrap())),

// An associated type names will be missing for an RPITIT. It will
// later be given a name with `synthetic` in it, if necessary.
DefKind::AssocTy => DefPathData::TypeNs(name),

// It's not exactly an anon const, but wrt DefPathData, there
// is no difference.
DefKind::Static { nested: true, .. } => DefPathData::AnonConst,
Expand Down
22 changes: 13 additions & 9 deletions compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,9 @@ pub enum DefPathData {
Use,
/// A global asm item.
GlobalAsm,
/// Something in the type namespace.
TypeNs(Symbol),
/// Something in the type namespace. Will be empty for RPITIT associated
/// types, which are given a synthetic name later, if necessary.
TypeNs(Option<Symbol>),
/// Something in the value namespace.
ValueNs(Symbol),
/// Something in the macro namespace.
Expand Down Expand Up @@ -410,8 +411,9 @@ impl DefPathData {
pub fn get_opt_name(&self) -> Option<Symbol> {
use self::DefPathData::*;
match *self {
TypeNs(name) if name == kw::Empty => None,
TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => Some(name),
TypeNs(name) => name,

ValueNs(name) | MacroNs(name) | LifetimeNs(name) => Some(name),

Impl | ForeignMod | CrateRoot | Use | GlobalAsm | Closure | Ctor | AnonConst
| OpaqueTy => None,
Expand All @@ -421,12 +423,14 @@ impl DefPathData {
pub fn name(&self) -> DefPathDataName {
use self::DefPathData::*;
match *self {
TypeNs(name) if name == kw::Empty => {
DefPathDataName::Anon { namespace: sym::synthetic }
}
TypeNs(name) | ValueNs(name) | MacroNs(name) | LifetimeNs(name) => {
DefPathDataName::Named(name)
TypeNs(name) => {
if let Some(name) = name {
DefPathDataName::Named(name)
} else {
DefPathDataName::Anon { namespace: sym::synthetic }
}
}
ValueNs(name) | MacroNs(name) | LifetimeNs(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 Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
// the children of the visible parent (as was done when computing
// `visible_parent_map`), looking for the specific child we currently have and then
// have access to the re-exported name.
DefPathData::TypeNs(ref mut name) if Some(visible_parent) != actual_parent => {
DefPathData::TypeNs(Some(ref mut name)) if Some(visible_parent) != actual_parent => {
// Item might be re-exported several times, but filter for the one
// that's public and whose identifier isn't `_`.
let reexport = self
Expand All @@ -575,7 +575,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
}
// Re-exported `extern crate` (#43189).
DefPathData::CrateRoot => {
data = DefPathData::TypeNs(self.tcx().crate_name(def_id.krate));
data = DefPathData::TypeNs(Some(self.tcx().crate_name(def_id.krate)));
}
_ => {}
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/ty/significant_drop_order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ fn true_significant_drop_ty<'tcx>(

match key.disambiguated_data.data {
rustc_hir::definitions::DefPathData::CrateRoot => {
name_rev.push(tcx.crate_name(did.krate))
name_rev.push(tcx.crate_name(did.krate));
}
rustc_hir::definitions::DefPathData::TypeNs(symbol) => {
name_rev.push(symbol.unwrap());
}
rustc_hir::definitions::DefPathData::TypeNs(symbol) => name_rev.push(symbol),
_ => return None,
}
if let Some(parent) = key.parent {
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_ty_utils/src/assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,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);
// FIXME: `kw::Empty` gets special treatment by `DefPathData`'s methods.
let trait_assoc_ty = tcx.at(span).create_def(trait_def_id, Some(kw::Empty), DefKind::AssocTy);
// No name because this is a synthetic associated type.
let trait_assoc_ty = tcx.at(span).create_def(trait_def_id, None, DefKind::AssocTy);

let local_def_id = trait_assoc_ty.def_id();
let def_id = local_def_id.to_def_id();
Expand Down Expand Up @@ -305,9 +305,8 @@ fn associated_type_for_impl_trait_in_impl(
hir::FnRetTy::DefaultReturn(_) => tcx.def_span(impl_fn_def_id),
hir::FnRetTy::Return(ty) => ty.span,
};
// FIXME: `kw::Empty` gets special treatment by `DefPathData`'s methods.
let impl_assoc_ty =
tcx.at(span).create_def(impl_local_def_id, Some(kw::Empty), DefKind::AssocTy);
// No name because this is a synthetic associated type.
let impl_assoc_ty = tcx.at(span).create_def(impl_local_def_id, None, DefKind::AssocTy);

let local_def_id = impl_assoc_ty.def_id();
let def_id = local_def_id.to_def_id();
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3489,7 +3489,7 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
// a::b::c ::d::sym refers to
// e::f::sym:: ::
// result should be super::super::super::super::e::f
if let DefPathData::TypeNs(s) = l {
if let DefPathData::TypeNs(Some(s)) = l {
path.push(s.to_string());
}
if let DefPathData::TypeNs(_) = r {
Expand All @@ -3500,7 +3500,7 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
// a::b::sym:: :: refers to
// c::d::e ::f::sym
// when looking at `f`
Left(DefPathData::TypeNs(sym)) => path.push(sym.to_string()),
Left(DefPathData::TypeNs(Some(sym))) => path.push(sym.to_string()),
// consider:
// a::b::c ::d::sym refers to
// e::f::sym:: ::
Expand All @@ -3514,7 +3514,7 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St
// `super` chain would be too long, just use the absolute path instead
once(String::from("crate"))
.chain(to.data.iter().filter_map(|el| {
if let DefPathData::TypeNs(sym) = el.data {
if let DefPathData::TypeNs(Some(sym)) = el.data {
Some(sym.to_string())
} else {
None
Expand Down
12 changes: 0 additions & 12 deletions tests/crashes/133426.rs

This file was deleted.

20 changes: 20 additions & 0 deletions tests/ui/lowering/no-name-for-DefPath-issue-133426.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! Test for the crash in #133426, caused by an empty symbol being used for a
//! type name.
#![allow(incomplete_features)]
#![feature(never_patterns)]

fn a(
_: impl Iterator<
Item = [(); {
match *todo!() { ! }; //~ ERROR type `!` cannot be dereferenced
}],
>,
) {
}

fn b(_: impl Iterator<Item = { match 0 { ! } }>) {}
//~^ ERROR associated const equality is incomplete
//~| ERROR expected type, found constant

fn main() {}

0 comments on commit 292031d

Please sign in to comment.