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.

Sign up for GitHub

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

Jump to bottom

Replacement PR for https://github.com/nim-lang/Nim/pull/23779 that #23793

Merged
merged 9 commits into from
Jul 7, 2024
Merged

Replacement PR for https://github.com/nim-lang/Nim/pull/23779 that #23793

merged 9 commits into from
Jul 7, 2024

Conversation

c-blake
Copy link
Contributor

@c-blake c-blake commented Jul 3, 2024

makes new hash the default, with an opt-out (& js-no-big-int) define. Also update changelog (& fix one typo).

Only really expect the chronos hash-order sensitive test to fail until they merge that PR and tag a new release.

@c-blake
Replacement PR for nim-lang#23779 that
b9c61d9 
makes new hash the default, with an opt-out (& js-no-big-int) define.
Also update changelog (& fix one typo).
@c-blake
Oops - give when true/false bodies.
903ae2b
@c-blake
Flip boolean here to match prior nimPreviewHashFarm condition.
465757f
@c-blake
Use not defined(nimHasJsBigIntBackend)) instead since Hash.sizeof=4
e6970cb 
on js even w/--jsbigint64:on property.
@c-blake
Make tests green by blocking the bad combination of the new default
09bcf83 
hash with `--jsbigint64:off` and eliminating this from test matrix.
For now leave seemingly unnecessary when nimvm's just commented out,
but there are obvious simplifications to maybe do before a merge.
@c-blake
Fix nim doc mode not having compile option. Make tests/stdlib/thashes
c192ed7 
consistent with lib/pure/hashes.nim
@c-blake
Minor amendments to 7 test matrices to pair up --jsbigint64:off with
26a3121 
the needed new define.
@c-blake
Table has long had == which here eliminates explicit hash-order via
6bfb772 
echo & output tests.
@c-blake
Copy link
Contributor Author

c-blake commented Jul 4, 2024

I think everything here is passing except for A) a now 2nd time for just me timeout failure on tests/vm/tslow_tables.nim and B) the chronos test failures on 4 platforms. Once @arnetheduck approves the CI workflow so that PR can be tested & merged & then once a new chronos is released, I plan to bump that timeout from 7 seconds to 9 or 10 seconds unless there is some objection. That will kick off a final test that should be all green.

@c-blake
Make test more faithful to original since in that only the someSeq
036d25d 
part of `Base` got compared (by echo output only).
@Araq Araq merged commit 4faa15f into nim-lang:devel Jul 7, 2024
16 of 18 checks passed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 7, 2024

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 4faa15f

Hint: mm: orc; opt: speed; options: -d:release
179108 lines; 9.401s; 664.625MiB peakmem

@ringabout ringabout mentioned this pull request Jul 9, 2024
Closed
@narimiran narimiran mentioned this pull request Jul 9, 2024
Closed
@c-blake c-blake deleted the flipFromOptInFarmToOptOut branch July 13, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@c-blake @Araq