Skip to content

Conversation

nnethercote
Copy link
Contributor

Some improvements in and around DefPathData, following on from #137977.

r? @spastorino

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 rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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 Apr 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 11, 2025

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
}
}
Copy link
Member

Choose a reason for hiding this comment

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

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),

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the if statement

@rust-log-analyzer

This comment has been minimized.

Give them their own symbol `anon_assoc`, as is done for all the other
anonymous `DefPathData` variants.
@@ -451,6 +446,7 @@ impl DefPathData {
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 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% certain about the name anon_assoc, happy to hear alternative suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

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.

@compiler-errors
Copy link
Member

r? compiler-errors @bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 11, 2025

📌 Commit cdf5b8d 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 Apr 11, 2025
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
@bors bors merged commit 2b54f9b into rust-lang:master Apr 11, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 11, 2025
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`
@nnethercote nnethercote deleted the tweak-DefPathData branch April 13, 2025 22:10
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PG-exploit-mitigations Project group: Exploit mitigations 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
Development

Successfully merging this pull request may close these issues.

7 participants