Merged
Conversation
No need to convert the `DefKind` to `DefPathData`, they're very similar types.
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.
rustbot
added
PG-exploit-mitigations
Collaborator
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_sanitizers cc @rcvalle |
Comment on lines
+273
to
+279
| DefKind::AssocTy => { | ||
| if let Some(name) = name { | ||
| DefPathData::TypeNs(name) | ||
| } else { | ||
| DefPathData::AnonAssocTy | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
I know it's a matter of taste, but I find the following more compact and as readable,feel free to ignore:
Suggested change
| DefKind::AssocTy => { | |
| if let Some(name) = name { | |
| DefPathData::TypeNs(name) | |
| } else { | |
| DefPathData::AnonAssocTy | |
| } | |
| } | |
| DefKind::AssocTy => name.map_or(DefPathData::AnonAssocTy, DefPathData::TypeNs), |
Contributor
There was a problem hiding this comment.
I prefer the if statement
This comment has been minimized.
This comment has been minimized.
Give them their own symbol `anon_assoc`, as is done for all the other anonymous `DefPathData` variants.
nnethercote
force-pushed
the
tweak-DefPathData
branch
from
9e29ae3 to
cdf5b8d
Compare
nnethercote
commented
Apr 11, 2025
| Ctor => DefPathDataName::Anon { namespace: sym::constructor }, | ||
| AnonConst => DefPathDataName::Anon { namespace: sym::constant }, | ||
| OpaqueTy => DefPathDataName::Anon { namespace: sym::opaque }, | ||
| AnonAssocTy => DefPathDataName::Anon { namespace: sym::anon_assoc }, |
Contributor
Author
There was a problem hiding this comment.
I'm not 100% certain about the name anon_assoc, happy to hear alternative suggestions.
Contributor
There was a problem hiding this comment.
I'm not a huge fan of the name, but not enough to try to think of a new name. Ideally we wouldn't render this at all in diagnostics, but special casing all diagnostics that render paths would be difficult b/c we'd need to work out an alternative to rendering the path of the anonymous GAT.
Contributor
|
r? compiler-errors @bors r+ rollup |
Collaborator
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 11, 2025
Rollup of 7 pull requests Successful merges: - rust-lang#137835 (Use `BinOp::Cmp` for `iNN::signum`) - rust-lang#139584 (Avoid a reverse map that is only used in diagnostics paths) - rust-lang#139638 (Cleanup the `InstSimplify` MIR transformation) - rust-lang#139653 (Handle a negated literal in `eat_token_lit`.) - rust-lang#139662 (Tweak `DefPathData`) - rust-lang#139664 (Reuse address-space computation from global alloc) - rust-lang#139687 (Add spastorino to users_on_vacation) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 11, 2025
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
…ompiler-errors Tweak `DefPathData` Some improvements in and around `DefPathData`, following on from rust-lang#137977. r? `@spastorino`
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Some improvements in and around
DefPathData, following on from #137977.r? @spastorino