Doc comments: use std::unordered_map #11113
Conversation
Co-authored-by: Eelco Dolstra <[email protected]>
src/libexpr/pos-idx.hh
Outdated
| @@ -2,12 +2,15 @@ | |||
|
|
|||
| #include <cinttypes> | |||
|
|
|||
| #include "util.hh" | |||
There was a problem hiding this comment.
Why is this included?
Can't seem to tell.
There was a problem hiding this comment.
Thanks for asking. It was for hash_combine, but that wasn't obvious at all.
I've split that into its own header, partly because "util" is a bad scope for a header, but also for the less subjective argument, from the commit message:
Splitting it out immediately answers questions like [this], without increasing the number of compilation units.
I did consider using boost::hash_combine instead, but it doesn't seem to be quite as capable, accepting only two arguments.
Splitting it out immediately answers questions like [this], without increasing the number of compilation units. I did consider using boost::hash_combine instead, but it doesn't seem to be quite as capable, accepting only two arguments. [this]: #11113 (comment)
Splitting it out immediately answers questions like [this], without increasing the number of compilation units. I did consider using boost::hash_combine instead, but it doesn't seem to be quite as capable, accepting only two arguments. [this]: #11113 (comment)
roberth
force-pushed
the
doc-comment-unordered-map
branch
from
fcdcffd to
d0e9878
Compare
src/libexpr/pos-idx.hh
Outdated
| size_t hash() const noexcept | ||
| { | ||
| size_t h = 854125; | ||
| hash_combine(h, id); |
There was a problem hiding this comment.
This hash_combine() seems unnecessary? We can just return the hash of id.
There was a problem hiding this comment.
In Java it's common practice to include a random number to represent the distinct type, but I think you're right.
That doesn't really apply here, because we don't have a "top" type like Java's Object that would create a need to avoid collisions when used in e.g. HashMap<String, Object>.
The closest thing we might have is std::variant. I think we can assume that it hashes its index number, which is probably better than salting each type anyway, because iirc it allows multiple distinct occurrences of the same type.
In C++ we don't need to salt the hash.
Motivation
As suggested in #11072 and now possible after #11092
Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.