Skip to content

Reduce kw::Empty usage, part 1 #137977

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 3 commits into from
Mar 7, 2025
Merged

Reduce kw::Empty usage, part 1 #137977

merged 3 commits into from
Mar 7, 2025

Conversation

nnethercote
Copy link
Contributor

This PR fixes some confusing kw::Empty usage, fixing a crash test along the way.

r? @spastorino

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 4, 2025
@nnethercote
Copy link
Contributor Author

Best reviewed one commit at a time.

@nnethercote nnethercote mentioned this pull request Mar 4, 2025
Open
@nnethercote nnethercote marked this pull request as ready for review March 4, 2025 05:35
@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2025

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in src/tools/compiletest

cc @jieyouxu

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

jieyouxu
jieyouxu reviewed Mar 4, 2025
View reviewed changes
src/tools/compiletest/src/runtest/crashes.rs
"crashtest no longer crashes/triggers ICE, horray! Please give it a meaningful \
"crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful \
Copy link
Member

Choose a reason for hiding this comment

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

I was like why did this PR get tagged as compiletest. 😆

spastorino
spastorino reviewed Mar 4, 2025
View reviewed changes
Copy link
Member

@spastorino spastorino left a comment

Choose a reason for hiding this comment

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

I've left a minor comment r=me doing whatever you feel is better there :).

compiler/rustc_hir/src/def.rs
| DefKind::TyParam
| DefKind::ExternCrate => DefPathData::TypeNs(name.unwrap()),
| DefKind::ExternCrate => DefPathData::TypeNs(Some(name.unwrap())),
Copy link
Member

Choose a reason for hiding this comment

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

why not just name here? I guess you want to make the thing explode if is None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. At the moment a None name only makes sense for DefKind::AssocTy, so I don't want to accept other combinations that aren't necessary.

@spastorino
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 5, 2025

📌 Commit 3ef8935 has been approved by spastorino

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 5, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 7, 2025
@jhpratt
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spa…
3a2325c 
…storino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? `@spastorino`
@jhpratt jhpratt mentioned this pull request Mar 7, 2025
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2025
@matthiaskrgr
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spa…
6435304 
…storino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? ``@spastorino``
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 7, 2025
Closed
@jhpratt
Copy link
Member

jhpratt commented Mar 7, 2025

@bors r-

#138152 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 7, 2025
@nnethercote
Fix a typo in the crashtest output.
e7bea57
@nnethercote
Pass Option<Symbol> to def_path_data/create_def methods.
7943932 
It's clearer than using `kw::Empty` to mean `None`.
@nnethercote
Make synthetic RPITIT assoc ty name handling more rigorous.
af92a33 
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 rust-lang#133426.
@nnethercote
Copy link
Contributor Author

I rebased.

@bors r=spastorino

@bors
Copy link
Collaborator

bors commented Mar 7, 2025

📌 Commit af92a33 has been approved by spastorino

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 7, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2025
@matthiaskrgr
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spa…
ca47b98 
…storino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? `@spastorino`
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 7, 2025
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2025
@matthiaskrgr
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spa…
1cc6a87 
…storino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? ``@spastorino``
This was referenced Mar 7, 2025
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
@bors
Auto merge of rust-lang#138177 - matthiaskrgr:rollup-y0ikkzz, r=matth…
f5a1ef7 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#134797 (Ergonomic ref counting)
 - rust-lang#137549 (Clean up various LLVM FFI things in codegen_llvm)
 - rust-lang#137977 (Reduce `kw::Empty` usage, part 1)
 - rust-lang#138042 (Suggest struct or union to add generic that impls trait)
 - rust-lang#138141 (tests: fix some typos in comment)
 - rust-lang#138150 (Streamline HIR intravisit `visit_id` calls for items)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0defc4f into rust-lang:master Mar 7, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2025
@rust-timer
Unrolled build for rust-lang#137977
0b6c069 
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spastorino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? ```@spastorino```
@nnethercote nnethercote deleted the less-kw-Empty-1 branch March 9, 2025 21:10
tautschnig added a commit to tautschnig/kani that referenced this pull request Mar 12, 2025
@tautschnig
Upgrade toolchain to nightly-2025-03-12
453d847 
Changes required due to
- rust-lang/rust#137977: Reduce `kw::Empty` usage, part 1

Resolves: model-checking#3932
@tautschnig tautschnig mentioned this pull request Mar 12, 2025
Merged
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request Mar 12, 2025
@tautschnig
Upgrade toolchain to nightly-2025-03-12 (#3933)
44a44e5 
Changes required due to
- rust-lang/rust#137977: Reduce `kw::Empty` usage, part 1

Resolves: #3932

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 20, 2025
@matthiaskrgr
Rollup merge of rust-lang#137977 - nnethercote:less-kw-Empty-1, r=spa…
eac092f 
…storino

Reduce `kw::Empty` usage, part 1

This PR fixes some confusing `kw::Empty` usage, fixing a crash test along the way.

r? ```@spastorino```
nnethercote added a commit to nnethercote/rust that referenced this pull request Apr 11, 2025
@nnethercote
Introduce DefPathData::AnonAssocTy.
9eca59a 
PR rust-lang#137977 changed `DefPathData::TypeNs` to contain `Option<Symbol>` to
account for RPITIT assoc types being anonymous. This commit changes it
back to `Symbol` and gives anonymous assoc types their own variant. It
makes things a bit nicer overall.
@nnethercote nnethercote mentioned this pull request Apr 11, 2025
Merged
jhpratt added a commit to jhpratt/rust that referenced this pull request Apr 11, 2025
@jhpratt
Rollup merge of rust-lang#139662 - nnethercote:tweak-DefPathData, r=c…
2b54f9b 
…ompiler-errors

Tweak `DefPathData`

Some improvements in and around `DefPathData`, following on from rust-lang#137977.

r? `@spastorino`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025
@rust-timer
Unrolled build for rust-lang#139662
f6d1242 
Rollup merge of rust-lang#139662 - nnethercote:tweak-DefPathData, r=compiler-errors

Tweak `DefPathData`

Some improvements in and around `DefPathData`, following on from rust-lang#137977.

r? `@spastorino`
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2025
@nnethercote
Introduce DefPathData::AnonAssocTy.
595c8f9 
PR rust-lang#137977 changed `DefPathData::TypeNs` to contain `Option<Symbol>` to
account for RPITIT assoc types being anonymous. This commit changes it
back to `Symbol` and gives anonymous assoc types their own variant. It
makes things a bit nicer overall.
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Apr 22, 2025
@jhpratt
Rollup merge of rust-lang#139662 - nnethercote:tweak-DefPathData, r=c…
9849319 
…ompiler-errors

Tweak `DefPathData`

Some improvements in and around `DefPathData`, following on from rust-lang#137977.

r? `@spastorino`
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@spastorino spastorino spastorino left review comments

@jieyouxu jieyouxu jieyouxu left review comments

Assignees

@spastorino spastorino

Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.
6 participants
@nnethercote @rustbot @spastorino @bors @jhpratt @jieyouxu