Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hash.requirements] clarify that Cpp17Hash does not imply stateless #7733

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jwakely
Copy link
Member

@jwakely jwakely commented Mar 13, 2025

A colleague was mislead into thinking that the Cpp17Hash requirements imply hash functions must be stateless, so that every h would produce the same value for h(k). I think we can dispel that just by clarifying the note, so that it doesn't seem to imply that only the value of k matters.

@jwakely
Copy link
Member Author

jwakely commented Mar 13, 2025

The normative wording before that note can also be interpreted to say that the value of h is not relevant, only k matters. So maybe we need an LWG issue here instead.

@jwakely
Copy link
Member Author

jwakely commented Mar 13, 2025

LWG2291 touched this last, but the problematic part was already there.

@@ -2076,7 +2076,7 @@
the program.
\begin{note}
Thus all evaluations of the expression \tcode{h(k)} with the
same value for \tcode{k} yield the same result for a given execution of the program.
same values for \tcode{h} and \tcode{k} yield the same result for a given execution of the program.
Copy link
Member

Choose a reason for hiding this comment

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

What if the evaluation h(k) changes h? Then, for the next evaluation, the precondition "same value of h" is not satisfied, but that's not in the spirit of the rule, I think. We should clarify that evaluation of h(k) should not change h.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants