Add note to documentation of HashSet::intersection

[nzrq]

The functionality of the std::collections::HashSet::intersection(...) method was slightly surprising to me so I wanted to take a sec to contribute to the documentation for this method.

I've added a Note: section if that is appropriate.

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) soon.

Please see the contribution instructions for more information.

[thomcc]

Ignore me.

Hm. While I don't think I'd encourage writing code that relied on this, the current implementation always provides items from self (and this is true of the BTreeSet::intersection function as well).

This behavior is consistent and possible to observe this with a totally reasonable Eq implementation¹, so even if I would personally prefer it be unspecified², I would not be surprised at all if people relied on this already (and I might even lean towards saying it's already de-facto specified because it's consistent, observable, and (probably) not really worth changing...)

But either way, it's not up to me. I think this needs an explicit T-libs-api decision.

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

Footnotes

1. ISTM that none of the requirements in the documentation of Eq indicate that it is invalid for an Eq impl to only consider a subset of fields. ↩

2. For example, I think it would likely be faster to iterate over (and yield elements from) whichever set is smaller, (for example, it would perform fewer lookups, which may be more efficient). That said, this sounds... situational at best, so IDK if I really think it's worth risking breakage over. ↩

[thomcc]

CC @Amanieu (who might have stronger opinions about the things we may want to promise here).

[the8472]

Hm. While I don't think I'd encourage writing code that relied on this, the current implementation always provides items from self (and this is true of the BTreeSet::intersection function as well).

😕❓

rust/library/std/src/collections/hash/set.rs

Lines 610 to 614 in 9a74608

	if self.len() <= other.len() {
	Intersection { iter: self.iter(), other }
	} else {
	Intersection { iter: other.iter(), other: self }
	}

[thomcc]

Wow, yep, my bad, I'm not sure what I was even looking at (... probably my lunch). So I don't think this needs API decision, although the currently assigned reviewer can take it.

[dtolnay]

Thanks!

[dtolnay]

@bors r+ rollup

[bors]

📌 Commit 7d114c7 has been approved by dtolnay