Skip to content

Conversation

peter-jerry-ye
Copy link
Collaborator

This PR makes the hash seed random on JavaScript backend, and cleanup the relevant tests.

This PR is broken into two steps:

  1. Migrate key.hash() to Hasher::new()..combine(key).finalize(). This is to make sure that the hash values are computed based on the hash combine with a seed, which is fixed at this stage. The tests based on the specific hash values are cleaned up.
  2. Make the seed in Hasher::new() random for JS backend. It will use crypto.getRandomValue if possible, and fallback to Math.random. The tests based on the fixed hash computations are cleaned up.

@peter-jerry-ye
Copy link
Collaborator Author

Hasher::new() -> key.hash

remove impl hash

@coveralls
Copy link
Collaborator

coveralls commented Sep 29, 2025

Pull Request Test Coverage Report for Build 1429

Details

  • 12 of 13 (92.31%) changed or added relevant lines in 6 files are covered.
  • 18 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.2%) to 89.198%

Changes Missing Coverage Covered Lines Changed/Added Lines %
builtin/hasher.mbt 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
immut/internal/sparse_array/sparse_array.mbt 1 94.19%
hashmap/json.mbt 3 0.0%
hashmap/utils.mbt 6 87.93%
hashset/hashset.mbt 8 88.14%
Totals Coverage Status
Change from base Build 1424: -0.2%
Covered Lines: 9265
Relevant Lines: 10387

💛 - Coveralls

@peter-jerry-ye
Copy link
Collaborator Author

@quake Would you like to review this PR?

@quake
Copy link
Contributor

quake commented Sep 29, 2025

@quake Would you like to review this PR?

LGTM

@peter-jerry-ye peter-jerry-ye enabled auto-merge (rebase) September 29, 2025 09:19
@peter-jerry-ye peter-jerry-ye merged commit d1a5dc7 into main Sep 29, 2025
14 checks passed
@peter-jerry-ye peter-jerry-ye deleted the zihang/make-hash-random branch September 29, 2025 09:25
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.

3 participants