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.

Sign up for GitHub

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

Jump to bottom

Reduce kw::Empty usage, part 2 #138347

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Reduce kw::Empty usage, part 2 #138347

wants to merge 4 commits into from
+41 −21

Conversation

nnethercote
Copy link
Contributor

A few small kw::Empty removals, or steps toward removals.

r? fmease

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@nnethercote
Copy link
Contributor Author

Best reviewed one commit at a time.

@bors rollup

@nnethercote
Copy link
Contributor Author

Another small step towards #137978.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
compiler-errors
compiler-errors approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me with grammar nits from those review comments

@petrochenkov
Copy link
Contributor

Right now I don't see what is the point of sym::dummy, considering that kw::Empty is already effectively a dummy symbol.
I guess it will depend on what the remaining uses of kw::Empty will be after you complete the whole kw::Empty reduction plan.

@nnethercote
Copy link
Contributor Author

If nothing else, it's a useful way to identify which kw::Empty uses have been inspected and confirmed to be genuine dummy uses.

@nnethercote
Convert a delayed bug to a bug.
763db5d 
This is never hit in the test suite.

At some point the check should be removed entirely. There are a million
places in the compiler where an empty symbol doesn't make sense, so a
check of this nature has almost zero value. But I'll leave it in place
for now just in case it gets hit by fuzzing or in the wild.
@nnethercote
Remove an unnecessary kw::Empty check.
fe04460 
Replace it with an assert for now, just in case it is reachable.
@nnethercote
Introduce sym::dummy and Ident::dummy.
0b2d706 
The idea is to identify cases of symbols/identifiers that are not
expected to be used. There isn't a perfectly sharp line between "dummy"
and "not dummy", but I think it's useful nonetheless.
@nnethercote
Convert a kw::Empty occurrence to sym::dummy.
4eadaff 
`adjust_ident_and_get_scope` returns the symbol it receives unchanged,
and the call site ignores the returned symbol, so this symbol is unused.
@nnethercote
Copy link
Contributor Author

I fixed the nits.

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Mar 11, 2025

📌 Commit 4eadaff has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
@jieyouxu
Rollup merge of rust-lang#138347 - nnethercote:less-kw-Empty-2, r=com…
15148c5 
…piler-errors

Reduce `kw::Empty` usage, part 2

A few small `kw::Empty` removals, or steps toward removals.

r? fmease
@jieyouxu jieyouxu mentioned this pull request Mar 12, 2025
Closed
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
@jieyouxu
Rollup merge of rust-lang#138347 - nnethercote:less-kw-Empty-2, r=com…
8901c0b 
…piler-errors

Reduce `kw::Empty` usage, part 2

A few small `kw::Empty` removals, or steps toward removals.

r? fmease
@jieyouxu jieyouxu mentioned this pull request Mar 12, 2025
Open
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
@bors
Auto merge of rust-lang#138383 - jieyouxu:rollup-ntnvzmn, r=jieyouxu
74d92df 
Rollup of 25 pull requests

Successful merges:

 - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`)
 - rust-lang#136842 (Add libstd support for Trusty targets)
 - rust-lang#137314 (change definitely unproductive cycles to error)
 - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.)
 - rust-lang#137621 (Add std support to cygwin target)
 - rust-lang#137701 (Convert `ShardedHashMap` to use `hashbrown::HashTable`)
 - rust-lang#138109 (make precise capturing args in rustdoc Json typed)
 - rust-lang#138161 (Add PeekMut::refresh)
 - rust-lang#138162 (Update the standard library to Rust 2024)
 - rust-lang#138174 (Elaborate trait assumption in `receiver_is_dispatchable`)
 - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj)
 - rust-lang#138269 (uefi: fs: Implement FileType, FilePermissions and FileAttr)
 - rust-lang#138313 (Update books)
 - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js)
 - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more)
 - rust-lang#138333 (Rebuild llvm spuriously less frequently)
 - rust-lang#138343 (Enable `f16` tests for `powf`)
 - rust-lang#138345 (Some autodiff cleanups)
 - rust-lang#138346 (naked functions: on windows emit `.endef` without the symbol name)
 - rust-lang#138347 (Reduce `kw::Empty` usage, part 2)
 - rust-lang#138360 (Fix false-positive in `expr_or_init` and in the `invalid_from_utf8` lint)
 - rust-lang#138371 (Update compiletest's `has_asm_support` to match rustc)
 - rust-lang#138372 (Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`)
 - rust-lang#138376 (Item-related cleanups)
 - rust-lang#138377 (Remove unnecessary lifetime from `PatInfo`.)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcol mcol mcol left review comments

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@nnethercote @rustbot @rust-log-analyzer @petrochenkov @bors @compiler-errors @mcol @fmease