Remove kw::Empty uses in rustdoc
#139846
Merged
Remove kw::Empty uses in rustdoc
#139846
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some `unwrap` uses here, but they are on paths involving item kinds that are known to have an identifier.
Again by using `Option<Symbol>` to represent "no name".
`sym::dummy` also appears to work.
rustbot
added
A-rustdoc-json
|
Best reviewed one commit at a time. |
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 15, 2025
Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`
|
☀️ Try build successful - checks-actions |
src/librustdoc/clean/mod.rs
Outdated
Comment on lines
2794
to
2798
| let mut name = if renamed.is_some() { | ||
| renamed | ||
| } else { | ||
| cx.tcx.hir_opt_name(item.hir_id()) | ||
| }; |
There was a problem hiding this comment.
No need to keep an Option around, right? If so:
Suggested change
| let mut name = if renamed.is_some() { | |
| renamed | |
| } else { | |
| cx.tcx.hir_opt_name(item.hir_id()) | |
| }; | |
| let mut name = renamed.unwrap_or_else(|| cx.tcx.hir_opt_name(item.hir_id()).unwrap()); |
There was a problem hiding this comment.
That doesn't work because he hir_opt_name can return None. So it must be an Option, and then lower down we only unwrap it for certain item kinds that always have an ident.
There was a problem hiding this comment.
Right. I have some ideas on how to improve it but for now let's move on.
GuillaumeGomez
approved these changes
Apr 16, 2025
|
Thanks! @bors r+ rollup |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 16, 2025
…illaumeGomez Remove `kw::Empty` uses in rustdoc Helps with rust-lang#137978. r? `@GuillaumeGomez`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 17, 2025
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139084 (hygiene: Rename semi-transparent to semi-opaque) - rust-lang#139236 (Use a session counter to make anon dep nodes unique) - rust-lang#139650 (Fix `register_group_alias` for tools) - rust-lang#139770 (Rename `LifetimeName` as `LifetimeKind`.) - rust-lang#139846 (Remove `kw::Empty` uses in rustdoc) - rust-lang#139891 (Include optional dso_local marker for functions in `enum-match.rs`) - rust-lang#139908 (parser: Remove old diagnostic notes for type ascription syntax) - rust-lang#139917 (fix for multiple `#[repr(align(N))]` on functions) Failed merges: - rust-lang#139615 (Remove `name_or_empty`) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 17, 2025
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``
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 17, 2025
…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.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 17, 2025
…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.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Apr 18, 2025
…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.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 18, 2025
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.
ChrisDenton
added a commit
to ChrisDenton/rust
that referenced
this pull request
Apr 19, 2025
…_renamed_item, r=nnethercote Improve `clean_maybe_renamed_item` function code a bit Follow-up of rust-lang#139846. This is what I tried to say in there: the `name` variable can be unwrapped in most cases so better do it directly once and for all if possible and move the cases where it's not possible above. r? `@nnethercote`
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 20, 2025
Rollup merge of rust-lang#140008 - GuillaumeGomez:cleanup-clean_maybe_renamed_item, r=nnethercote Improve `clean_maybe_renamed_item` function code a bit Follow-up of rust-lang#139846. This is what I tried to say in there: the `name` variable can be unwrapped in most cases so better do it directly once and for all if possible and move the cases where it's not possible above. r? `@nnethercote`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Helps with #137978.
r? @GuillaumeGomez