Too much `kw::Empty` usage #137978

nnethercote · 2025-03-04T04:54:48Z

kw::Empty is the name of the empty Symbol. It's used a lot. Most places it's used, it's hard to tell if it means "empty symbol" or "no symbol". I.e. it more or less reinvents the null pointer, something that Rust was supposed to get rid of.

I think a lot of the uses could be removed by using Option<Symbol> instead of Symbol, making the "no symbol" cases much clearer.

The text was updated successfully, but these errors were encountered:

nnethercote · 2025-03-04T04:56:55Z

#137977 is the first step here. The backstory: I grepped for kw::Empty, looked at the very first occurrence in the output, worked out how to eliminate it, removing 17 kw::Empty occurrences, and I unintentionally fixed a crashtest in the process. A promising start.

…try> Move `hir::Item::ident` into `hir::ItemKind`. `hir::Item` has an `ident` field. - It's always non-empty for these item kinds: `ExternCrate`, `Static`, `Const`, `Fn`, `Macro`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`, Trait`, TraitAalis`. - It's always empty for these item kinds: `ForeignMod`, `GlobalAsm`, `Impl`. - For `Use`, it is non-empty for `UseKind::Single` and empty for `UseKind::{Glob,ListStem}`. All of this is quite non-obvious; the only documentation is a single comment saying "The name might be a dummy name in case of anonymous items". Some sites that handle items check for an empty ident, some don't. This is a very C-like way of doing things, but this is Rust, we have sum types, we can do this properly and never forget to check for the exceptional case and never YOLO possibly empty identifiers (or possibly dummy spans) around and hope that things will work out. This is step towards `kw::Empty` elimination (rust-lang#137978). r? `@ghost`

nnethercote · 2025-03-16T20:23:13Z

#138482 fixed some HIR printing bugs. That PR didn't reduce kw::Empty usage, but I found the bugs while working on related kw::Empty reduction. More evidence that kw::Empty makes it easy to get things wrong.

…=fmease Move `hir::Item::ident` into `hir::ItemKind`. `hir::Item` has an `ident` field. - It's always non-empty for these item kinds: `ExternCrate`, `Static`, `Const`, `Fn`, `Macro`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`, Trait`, TraitAalis`. - It's always empty for these item kinds: `ForeignMod`, `GlobalAsm`, `Impl`. - For `Use`, it is non-empty for `UseKind::Single` and empty for `UseKind::{Glob,ListStem}`. All of this is quite non-obvious; the only documentation is a single comment saying "The name might be a dummy name in case of anonymous items". Some sites that handle items check for an empty ident, some don't. This is a very C-like way of doing things, but this is Rust, we have sum types, we can do this properly and never forget to check for the exceptional case and never YOLO possibly empty identifiers (or possibly dummy spans) around and hope that things will work out. This is step towards `kw::Empty` elimination (rust-lang#137978). r? `@fmease`

Rollup merge of rust-lang#138384 - nnethercote:hir-ItemKind-idents, r=fmease Move `hir::Item::ident` into `hir::ItemKind`. `hir::Item` has an `ident` field. - It's always non-empty for these item kinds: `ExternCrate`, `Static`, `Const`, `Fn`, `Macro`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`, Trait`, TraitAalis`. - It's always empty for these item kinds: `ForeignMod`, `GlobalAsm`, `Impl`. - For `Use`, it is non-empty for `UseKind::Single` and empty for `UseKind::{Glob,ListStem}`. All of this is quite non-obvious; the only documentation is a single comment saying "The name might be a dummy name in case of anonymous items". Some sites that handle items check for an empty ident, some don't. This is a very C-like way of doing things, but this is Rust, we have sum types, we can do this properly and never forget to check for the exceptional case and never YOLO possibly empty identifiers (or possibly dummy spans) around and hope that things will work out. This is step towards `kw::Empty` elimination (rust-lang#137978). r? `@fmease`

nnethercote · 2025-03-19T22:12:39Z

#138685 removes some more kw::Empty uses.

nnethercote · 2025-03-19T23:03:35Z

#138384 removed some dubious use of kw::Empty to give an empty identifier to anonymous items (e.g. Impl, ForeignMod).

…piler-errors Reduce `kw::Empty` usage, part 3 Remove some more `kw::Empty` uses, in support of rust-lang#137978. r? `@davidtwco`

Rollup merge of rust-lang#138924 - nnethercote:less-kw-Empty-3, r=compiler-errors Reduce `kw::Empty` usage, part 3 Remove some more `kw::Empty` uses, in support of rust-lang#137978. r? `@davidtwco`

…, r=GuillaumeGomez rustdoc: remove useless `Symbol::is_empty` checks. There are a number of `is_empty` checks that can never fail. This commit removes them, in support of rust-lang#137978. r? `@GuillaumeGomez`

Rollup merge of rust-lang#138917 - nnethercote:rustdoc-remove-useless, r=GuillaumeGomez rustdoc: remove useless `Symbol::is_empty` checks. There are a number of `is_empty` checks that can never fail. This commit removes them, in support of rust-lang#137978. r? `@GuillaumeGomez`

nnethercote · 2025-03-28T02:05:33Z

Another notable one. hir::Lifetime::ident was sometimes empty for elided lifetimes and sometimes '_, it was very confusing and confused. #138965 cleaned this up a lot, eliminating the use of empty lifetime names and made some minor HIR pretty-printing improvements along the way.

…me, r=lcnr Remove `kw::Empty` uses from `hir::Lifetime::ident` `hir::Lifetime::ident` is sometimes set to `kw::Empty` and it's really confusing. This PR stops that. Helps with rust-lang#137978. r? `@lcnr`

Add new `PatKind::Missing` variants To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with rust-lang#137978. Details in the individual commits. r? `@ghost`

…rochenkov Reduce kw::Empty usage, part 4 Another step towards rust-lang#137978. r? `@petrochenkov`

Rollup merge of rust-lang#139039 - nnethercote:less-kw-Empty-4, r=petrochenkov Reduce kw::Empty usage, part 4 Another step towards rust-lang#137978. r? `@petrochenkov`

…-obk Add new `PatKind::Missing` variants To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with rust-lang#137978. Details in the individual commits. r? `@oli-obk`

…mpiler-errors Don't use empty trait names Helps with rust-lang#137978. Details in individual commits. r? ``@davidtwco``

nnethercote · 2025-04-10T05:32:52Z

#139615 fixes some cases where empty identifiers leaked through to some error messages such as this:

error: unrecognized field name ``

…mpiler-errors Don't use empty trait names Helps with rust-lang#137978. Details in individual commits. r? ```@davidtwco```

Rollup merge of rust-lang#139568 - nnethercote:empty-trait-name, r=compiler-errors Don't use empty trait names Helps with rust-lang#137978. Details in individual commits. r? ```@davidtwco```

This contributes towards rust-lang/rust#137978. changelog: none

To accurately reflect that RPITIT assoc items don't have a name. This avoids the use of `kw::Empty` to mean "no name", which is error prone. Helps with rust-lang#137978.

These were low value even before rust-lang#137978 resulted in empty names being used much less. (Why check for non-emptiness in these three places? There are thousands of places in the compiler you could check.)

Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`

…-errors Reduce kw::Empty usage, part 5 Another step towards rust-lang#137978. r? `@davidtwco`

Rollup merge of rust-lang#139848 - nnethercote:kw-Empty-5, r=compiler-errors Reduce kw::Empty usage, part 5 Another step towards rust-lang#137978. r? `@davidtwco`

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

…illaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`

…illaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? ``@GuillaumeGomez``

Rollup merge of rust-lang#139846 - nnethercote:kw-Empty-rustdoc, r=GuillaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? ``@GuillaumeGomez``

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

…ng, r=GuillaumeGomez rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) **(0)** PR rust-lang#136411 aimed to stop rendering unnamed params of fn ptr types as underscores in the common case (e.g., `fn(_: i32)` → `fn(i32)`) to make the rendered output stylistically more conventional. **(0.a)** However, since the cleaning fn that the PR modified is also used for lowering the HIR params of foreign fns and required assoc fns in traits, it accidentally butchered the rendering of the latter two: ```rs pub trait Trait { fn assoc_fn(_: i32); } // as well as (Rust 2015 only): fn assoc_fn(i32); unsafe extern "C" { pub fn foreign_fn(_: i32); } // Since 1.86 the fns above gets mis-rendered as: pub fn assoc_fn(: i32) // <-- BUTCHERED pub unsafe extern "C" fn foreign_fn(: i32) // <-- BUTCHERED ``` **(0.b)** Furthermore, it broke parity with middle cleaning (which includes inlined cross-crate re-exports) re-regressing parts of rust-lang#44306 I once fixed in PR rust-lang#103885. **(1)** Lastly, PR rust-lang#139035 introduced an ICE triggered by the following input file: ```rs trait Trait { fn anon(()) {} } // internal error: entered unreachable code ``` --- This PR fixes all of these regressions and in the first commit renames several types and fns to be more ~~correct~~ descriptive and legible. ~~It also refactors `Param.name` to be of type `Option<Symbol>` instead `Symbol` (where `None` ~ `kw::Empty`), so rendering mistakes like that can no longer creep in like that (ignoring tests). CC rust-lang#137978.~~ Independently done in PR rust-lang#139846 a day prior.

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

…ng, r=GuillaumeGomez rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) **(0)** PR rust-lang#136411 aimed to stop rendering unnamed params of fn ptr types as underscores in the common case (e.g., `fn(_: i32)` → `fn(i32)`) to make the rendered output stylistically more conventional. **(0.a)** However, since the cleaning fn that the PR modified is also used for lowering the HIR params of foreign fns and required assoc fns in traits, it accidentally butchered the rendering of the latter two: ```rs pub trait Trait { fn assoc_fn(_: i32); } // as well as (Rust 2015 only): fn assoc_fn(i32); unsafe extern "C" { pub fn foreign_fn(_: i32); } // Since 1.86 the fns above gets mis-rendered as: pub fn assoc_fn(: i32) // <-- BUTCHERED pub unsafe extern "C" fn foreign_fn(: i32) // <-- BUTCHERED ``` **(0.b)** Furthermore, it broke parity with middle cleaning (which includes inlined cross-crate re-exports) re-regressing parts of rust-lang#44306 I once fixed in PR rust-lang#103885. **(1)** Lastly, PR rust-lang#139035 introduced an ICE triggered by the following input file: ```rs trait Trait { fn anon(()) {} } // internal error: entered unreachable code ``` --- This PR fixes all of these regressions and in the first commit renames several types and fns to be more ~~correct~~ descriptive and legible. ~~It also refactors `Param.name` to be of type `Option<Symbol>` instead `Symbol` (where `None` ~ `kw::Empty`), so rendering mistakes like that can no longer creep in like that (ignoring tests). CC rust-lang#137978.~~ Independently done in PR rust-lang#139846 a day prior.

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

…ng, r=GuillaumeGomez rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) **(0)** PR rust-lang#136411 aimed to stop rendering unnamed params of fn ptr types as underscores in the common case (e.g., `fn(_: i32)` → `fn(i32)`) to make the rendered output stylistically more conventional. **(0.a)** However, since the cleaning fn that the PR modified is also used for lowering the HIR params of foreign fns and required assoc fns in traits, it accidentally butchered the rendering of the latter two: ```rs pub trait Trait { fn assoc_fn(_: i32); } // as well as (Rust 2015 only): fn assoc_fn(i32); unsafe extern "C" { pub fn foreign_fn(_: i32); } // Since 1.86 the fns above gets mis-rendered as: pub fn assoc_fn(: i32) // <-- BUTCHERED pub unsafe extern "C" fn foreign_fn(: i32) // <-- BUTCHERED ``` **(0.b)** Furthermore, it broke parity with middle cleaning (which includes inlined cross-crate re-exports) re-regressing parts of rust-lang#44306 I once fixed in PR rust-lang#103885. **(1)** Lastly, PR rust-lang#139035 introduced an ICE triggered by the following input file: ```rs trait Trait { fn anon(()) {} } // internal error: entered unreachable code ``` --- This PR fixes all of these regressions and in the first commit renames several types and fns to be more ~~correct~~ descriptive and legible. ~~It also refactors `Param.name` to be of type `Option<Symbol>` instead `Symbol` (where `None` ~ `kw::Empty`), so rendering mistakes like that can no longer creep in like that (ignoring tests). CC rust-lang#137978.~~ Independently done in PR rust-lang#139846 a day prior.

Rollup merge of rust-lang#139913 - fmease:rustdoc-fix-fn-param-handling, r=GuillaumeGomez rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) **(0)** PR rust-lang#136411 aimed to stop rendering unnamed params of fn ptr types as underscores in the common case (e.g., `fn(_: i32)` → `fn(i32)`) to make the rendered output stylistically more conventional. **(0.a)** However, since the cleaning fn that the PR modified is also used for lowering the HIR params of foreign fns and required assoc fns in traits, it accidentally butchered the rendering of the latter two: ```rs pub trait Trait { fn assoc_fn(_: i32); } // as well as (Rust 2015 only): fn assoc_fn(i32); unsafe extern "C" { pub fn foreign_fn(_: i32); } // Since 1.86 the fns above gets mis-rendered as: pub fn assoc_fn(: i32) // <-- BUTCHERED pub unsafe extern "C" fn foreign_fn(: i32) // <-- BUTCHERED ``` **(0.b)** Furthermore, it broke parity with middle cleaning (which includes inlined cross-crate re-exports) re-regressing parts of rust-lang#44306 I once fixed in PR rust-lang#103885. **(1)** Lastly, PR rust-lang#139035 introduced an ICE triggered by the following input file: ```rs trait Trait { fn anon(()) {} } // internal error: entered unreachable code ``` --- This PR fixes all of these regressions and in the first commit renames several types and fns to be more ~~correct~~ descriptive and legible. ~~It also refactors `Param.name` to be of type `Option<Symbol>` instead `Symbol` (where `None` ~ `kw::Empty`), so rendering mistakes like that can no longer creep in like that (ignoring tests). CC rust-lang#137978.~~ Independently done in PR rust-lang#139846 a day prior.

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jdonszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 4, 2025

nnethercote self-assigned this Mar 4, 2025

nnethercote added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-technical-debt Area: Internal cleanup work labels Mar 4, 2025

This was referenced Mar 11, 2025

Reduce kw::Empty usage, part 2 #138347

Merged

Move hir::Item::ident into hir::ItemKind. #138384

Merged

nnethercote mentioned this issue Mar 14, 2025

Fix HIR printing of parameters #138482

Merged

This was referenced Mar 25, 2025

Reduce kw::Empty usage, part 3 #138924

Merged

rustdoc: remove useless Symbol::is_empty checks. #138917

Merged

This was referenced Mar 27, 2025

Add new PatKind::Missing variants #139035

Merged

Remove kw::Empty uses from hir::Lifetime::ident #138965

Merged

Remove kw::Empty uses from rustc_middle. #138926

Merged

Reduce kw::Empty usage, part 4 #139039

Merged

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 31, 2025

Rollup merge of rust-lang#139039 - nnethercote:less-kw-Empty-4, r=pet…

2f5d548

…rochenkov Reduce kw::Empty usage, part 4 Another step towards rust-lang#137978. r? `@petrochenkov`

Zalathar added a commit to Zalathar/rust that referenced this issue Apr 10, 2025

Rollup merge of rust-lang#139568 - nnethercote:empty-trait-name, r=co…

6c891d6

…mpiler-errors Don't use empty trait names Helps with rust-lang#137978. Details in individual commits. r? ``@davidtwco``

nnethercote mentioned this issue Apr 10, 2025

Remove name_or_empty #139615

Merged

nnethercote mentioned this issue Apr 10, 2025

Avoid some uses of empty identifiers rust-lang/rust-clippy#14580

Merged

nnethercote mentioned this issue Apr 10, 2025

Avoid empty identifiers for delegate params and args. #139614

Merged

github-merge-queue bot pushed a commit to rust-lang/rust-clippy that referenced this issue Apr 11, 2025

Avoid some uses of empty identifiers (#14580)

0cd5b62

This contributes towards rust-lang/rust#137978. changelog: none

nnethercote mentioned this issue Apr 15, 2025

Remove kw::Empty uses in rustdoc #139846

Merged

nnethercote mentioned this issue Apr 15, 2025

Reduce kw::Empty usage, part 5 #139848

Merged

bors added a commit to rust-lang-ci/rust that referenced this issue Apr 15, 2025

Auto merge of rust-lang#139846 - nnethercote:kw-Empty-rustdoc, r=<try>

54da544

Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 15, 2025

Rollup merge of rust-lang#139848 - nnethercote:kw-Empty-5, r=compiler…

2144c94

…-errors Reduce kw::Empty usage, part 5 Another step towards rust-lang#137978. r? `@davidtwco`

rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 15, 2025

Unrolled build for rust-lang#139848

2962600

Rollup merge of rust-lang#139848 - nnethercote:kw-Empty-5, r=compiler-errors Reduce kw::Empty usage, part 5 Another step towards rust-lang#137978. r? `@davidtwco`

fmease mentioned this issue Apr 16, 2025

rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) #139913

Merged

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 16, 2025

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jd…

8150743

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 16, 2025

Rollup merge of rust-lang#139846 - nnethercote:kw-Empty-rustdoc, r=Gu…

76c46ed

…illaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 16, 2025

Rollup merge of rust-lang#139846 - nnethercote:kw-Empty-rustdoc, r=Gu…

fb3338c

…illaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? ``@GuillaumeGomez``

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 17, 2025

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jd…

e4a8af0

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 17, 2025

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jd…

f6008db

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? `@jdonszelmann`

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 18, 2025

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jd…

540fb22

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 18, 2025

Unrolled build for rust-lang#139615

756012b

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jdonszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 22, 2025

Rollup merge of rust-lang#139615 - nnethercote:rm-name_or_empty, r=jd…

ed892e7

…onszelmann Remove `name_or_empty` Another step towards rust-lang#137978. r? ``@jdonszelmann``

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Too much `kw::Empty` usage #137978

Too much `kw::Empty` usage #137978

nnethercote commented Mar 4, 2025

nnethercote commented Mar 4, 2025 •

edited

Loading

nnethercote commented Mar 16, 2025

nnethercote commented Mar 19, 2025

nnethercote commented Mar 19, 2025

nnethercote commented Mar 28, 2025

nnethercote commented Apr 10, 2025

Too much kw::Empty usage #137978

Too much kw::Empty usage #137978

Comments

nnethercote commented Mar 4, 2025

nnethercote commented Mar 4, 2025 • edited Loading

nnethercote commented Mar 16, 2025

nnethercote commented Mar 19, 2025

nnethercote commented Mar 19, 2025

nnethercote commented Mar 28, 2025

nnethercote commented Apr 10, 2025

Too much `kw::Empty` usage #137978

Too much `kw::Empty` usage #137978

nnethercote commented Mar 4, 2025 •

edited

Loading