search

pubgrub-rs / pubgrub

PubGrub version solving algorithm implemented in Rust
https://pubgrub-rs.github.io/pubgrub/pubgrub/
Mozilla Public License 2.0
337 stars 30 forks source link

impl: centralize use of hash sets/maps #182

Closed BurntSushi closed 5 months ago

BurntSushi commented 5 months ago

Previously, pubgrub used an alias for HashMap so that everything used the same implementation. But it didn't do the same for HashSet, and indeed, there were some uses of FxHashSet and std::collections::HashSet. Like for HashMap, we standard on the use of FxHashSet for everything.

Eh2406 commented 5 months ago

Do we ever iterate over those Sets? I don't think this is the source of the bugs you are trying to track down. Unless I missed something.

On the other hand, standardizing doesn't seem to hurt anything.

BurntSushi commented 5 months ago

Right, I have no idea whether these are the source of the non-determinism we're seeing. But as you say, I figured it'd be good to use the same hasher consistently everywhere (unless of course there's a good reason to do otherwise).

Eh2406 commented 5 months ago

Yes. On second thought I'm convinced. The fact that Set means different things in different modules is confusing.

weihanglo commented 5 months ago

Just a drive-by: clippy::disallowed-types can be configured to avoid misuse of HashSet/HashMap again.