recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher - #147171

Merged
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag
Nov 19, 2025
Merged

recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher#147171
bors merged 1 commit into
rust-lang:mainfrom
Qelxiros:hashmap_diag

Conversation

@Qelxiros

@QelxirosQelxiros commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

closes#147147

The suggestion span is wrong, but I'm not sure how to find the right one. fixed

I'm relatively new to the diagnostics ecosystem, so I'm not sure if span_help is the right choice. span_suggestion_* might be better, but the output from x test looks weird in that case.

@rustbotrustbot added 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 30, 2025
@rustbot

Copy link
Copy Markdown
Collaborator

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Comment on lines +10 to +13
note: the foreign item type `String` doesn't implement `BuildHasher`
--> $SRC_DIR/alloc/src/string.rs:LL:COL
|
= note: not implement `BuildHasher`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note to self, this probably needs improvement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could there be other tests that showcase when this help message could appear? For example when I try to insert, or when I try to create a new value of the HashSet type.

Also I think it might be better to put this under the less crowded tests/ui/hashmap directory.

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2025
@fee1-dead

Copy link
Copy Markdown
Member

Still waiting on other tests.

Also, use [@]rustbot ready if you want to resubmit for review

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

I've added a test for the insert case. It seems pretty representative of most instances of this error, since the HashSet bounds are on methods. If we want to lint (not error) on HashSet construction with a second parameter that doesn't impl BuildHasher (and I'm not convinced we do), that's a separate conversation.

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2025
Comment on lines +14 to +21
help: you might have intended to use a HashMap instead
--> $DIR/hashset_generics.rs:8:5
|
LL | #[derive(PartialEq)]
| --------- in this derive macro expansion
...
LL | pub parameters: HashSet<String, String>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use err.help() to avoid highlighting the same thing again?

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025
@bors

bors commented Oct 22, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #147957) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2025
Comment threadtests/ui/hashmap/hashset_generics.stderr
@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 5, 2025

@fee1-deadfee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

had one last nit, otherwise this is good to go!

View changes since this review

Comment on lines +3085 to +3097
if foreign_preds.iter().any(|&(root_pred, pred)| {
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(root_pred)) =
root_pred.kind().skip_binder()
&& let Some(root_adt) = root_pred.self_ty().ty_adt_def()
{
self.tcx.is_diagnostic_item(sym::HashSet, root_adt.did())
&& self.tcx.is_diagnostic_item(sym::BuildHasher, pred.def_id())
} else {
false
}
}) {
err.help("you might have intended to use a HashMap instead");
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be nice to extract this to a helper function that deduplicates the logic between this one and the one above, maybe named something like "suggest_hashmap_on_unsatisfied_hashset_buildhasher"

@rustbotrustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2025
@Qelxiros

Copy link
Copy Markdown
ContributorAuthor

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
@fee1-dead

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit 754a82d has been approved by fee1-dead

It is now in the queue for this repository.

@borsbors 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 Nov 18, 2025
bors added a commit that referenced this pull request Nov 19, 2025
Rollup of 7 pull requests
Successful merges:
- #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- #147421 (Add check if span is from macro expansion)
- #147521 (Make SIMD intrinsics available in `const`-contexts)
- #148201 (Start documenting autodiff activities)
- #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- #148798 (Match <OsString as Debug>::fmt to that of str)
- #149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 847c422 into rust-lang:mainNov 19, 2025
11 checks passed
@rustbotrustbot added this to the 1.93.0 milestone Nov 19, 2025
rust-timer added a commit that referenced this pull request Nov 19, 2025
Rollup merge of #147171 - Qelxiros:hashmap_diag, r=fee1-dead
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closes#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
github-actionsBot pushed a commit to rust-lang/miri that referenced this pull request Nov 20, 2025
Rollup of 7 pull requests
Successful merges:
- rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher)
- rust-lang/rust#147421 (Add check if span is from macro expansion)
- rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts)
- rust-lang/rust#148201 (Start documenting autodiff activities)
- rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`)
- rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str)
- rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book)
r? `@ghost`
`@rustbot` modify labels: rollup
github-actionsBot pushed a commit to model-checking/verify-rust-std that referenced this pull request Nov 30, 2025
recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher
closesrust-lang#147147
~The suggestion span is wrong, but I'm not sure how to find the right one.~ fixed
I'm relatively new to the diagnostics ecosystem, so I'm not sure if `span_help` is the right choice. `span_suggestion_*` might be better, but the output from `x test` looks weird in that case.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider suggesting using HashMap<String, String> instead of HashSet<String, String> when PartialEq derive macro is used

5 participants

@Qelxiros@rustbot@rust-log-analyzer@fee1-dead@bors