More deprecations in Pervasives; add Stdlib.Pair and Stdlib.Int.Ref - #7371
Merged
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Syntax Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.
| Benchmark suite | Current: 6a84bd1 | Previous: 3f06468 | Ratio |
|---|---|---|---|
Parse RedBlackTree.res - time/run |
1.8706953800000001 ms |
1.3461563799999998 ms |
1.39 |
Print RedBlackTree.res - time/run |
2.9356200866666664 ms |
1.8942094533333333 ms |
1.55 |
Print RedBlackTreeNoComments.res - time/run |
2.6982062666666664 ms |
1.7506641333333335 ms |
1.54 |
Parse Napkinscript.res - time/run |
63.02000473999999 ms |
42.33869618 ms |
1.49 |
Print Napkinscript.res - time/run |
101.28971613333333 ms |
57.47105978 ms |
1.76 |
Parse HeroGraphic.res - time/run |
7.623864753333333 ms |
5.736426893333333 ms |
1.33 |
Print HeroGraphic.res - time/run |
12.621982873333334 ms |
7.840572506666666 ms |
1.61 |
This comment was automatically generated by workflow using github-action-benchmark.
cknitt
changed the title
cknitt
commented
Mar 31, 2025
| /* Miscellaneous */ | ||
|
|
||
| @deprecated("This will be removed in v13") | ||
| type int32 = int |
Member
Author
tsnobip
reviewed
Apr 1, 2025
tsnobip
left a comment
tsnobip
left a comment
Member
There was a problem hiding this comment.
I'd just add equal and compare to Pair module and I think we're good to go!
Comment on lines
+111
to
+116
| module Ref = { | ||
| type t = ref<int> | ||
|
|
||
| external increment: ref<int> => unit = "%incr" | ||
| external decrement: ref<int> => unit = "%decr" | ||
| } |
Member
There was a problem hiding this comment.
what was the original reasoning behind Pervasives_mini?
Member
Author
There was a problem hiding this comment.
- Avoiding cycles because Pervasives had global stuff referencing other modules that is now in
Stdlib_Global. - Original idea was also to build Stdlib, Belt, Js separately based on a minimal set Pervasives_mini. Now Pervasives itself is reduced anyway with most stuff deprecated.
fhammerschmidt
approved these changes
Apr 1, 2025
fhammerschmidt
pushed a commit
that referenced
this pull request
Apr 4, 2025
…7371) * More deprecations in Pervasives * Add Stdlib.Pair * Add Int.Ref.increment/decrement * CHANGELOG * Get rid of Pervasives_mini * Fix CHANGELOG category * Add Pair.equal, Pair.compare
cristianoc
added a commit
that referenced
this pull request
Sep 4, 2026
None of these were designed for ReScript. %incr, %decr and %refget arrived with OCaml's Pervasives in the 2016 initial export and were carried unexamined through every stdlib reshuffle since. Int.Ref itself was created in April 2025 (#7371) not because anyone wanted it, but as somewhere for the Pervasives.incr deprecation to point; the primitives it wrapped were removed for v13 two weeks ago. Outside this repository, GitHub code search finds no user of either the externals or the API. What the primitive bought was unboxing: expanding at the call site kept the field write syntactically visible, so Lam_pass_eliminate_ref could still turn a local ref into a mutable variable. A call through an ordinary function cannot - the reference appears as a bare Lvar and the pass gives up. That is not special to increment. Its body is six nodes against a small_inline_size of five, and cross-module inlining is off, so the inliner cannot reach it. Writing the update directly does keep the unboxing, and is shorter than the call it replaces: Int.Ref.increment(v) -> v.contents = v.contents + 1 53 call sites across 30 test files change that way, and their generated JavaScript is byte-identical. Only two outputs move: Stdlib_Int loses an empty Ref object and its export, and test_incr_ref loses onExpression - added to pin that the primitive bound its argument before mentioning it twice, which has nothing left to test now that no expansion happens. Lambda.offset_ref and the Offset_ref builtin go with them. Nothing in lambda.ml now builds a term outside the constructors and the traversals. Int.Ref.t went too. It was a type alias for ref<int> introduced alongside the two functions, and with them gone the module held nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8g8qwBARAcvW9MyuKQq8H
cristianoc
added a commit
that referenced
this pull request
Sep 5, 2026
None of these were designed for ReScript. %incr, %decr and %refget arrived with OCaml's Pervasives in the 2016 initial export and were carried unexamined through every stdlib reshuffle since. Int.Ref itself was created in April 2025 (#7371) not because anyone wanted it, but as somewhere for the Pervasives.incr deprecation to point; the primitives it wrapped were removed for v13 two weeks ago. Outside this repository, GitHub code search finds no user of either the externals or the API. What the primitive bought was unboxing: expanding at the call site kept the field write syntactically visible, so Lam_pass_eliminate_ref could still turn a local ref into a mutable variable. A call through an ordinary function cannot - the reference appears as a bare Lvar and the pass gives up. That is not special to increment. Its body is six nodes against a small_inline_size of five, and cross-module inlining is off, so the inliner cannot reach it. Writing the update directly does keep the unboxing, and is shorter than the call it replaces: Int.Ref.increment(v) -> v.contents = v.contents + 1 53 call sites across 30 test files change that way, and their generated JavaScript is byte-identical. Only two outputs move: Stdlib_Int loses an empty Ref object and its export, and test_incr_ref loses onExpression - added to pin that the primitive bound its argument before mentioning it twice, which has nothing left to test now that no expansion happens. Lambda.offset_ref and the Offset_ref builtin go with them. Nothing in lambda.ml now builds a term outside the constructors and the traversals. Int.Ref.t went too. It was a type alias for ref<int> introduced alongside the two functions, and with them gone the module held nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8g8qwBARAcvW9MyuKQq8H
cristianoc
added a commit
that referenced
this pull request
Sep 5, 2026
None of these were designed for ReScript. %incr, %decr and %refget arrived with OCaml's Pervasives in the 2016 initial export and were carried unexamined through every stdlib reshuffle since. Int.Ref itself was created in April 2025 (#7371) not because anyone wanted it, but as somewhere for the Pervasives.incr deprecation to point; the primitives it wrapped were removed for v13 two weeks ago. Outside this repository, GitHub code search finds no user of either the externals or the API. What the primitive bought was unboxing: expanding at the call site kept the field write syntactically visible, so Lam_pass_eliminate_ref could still turn a local ref into a mutable variable. A call through an ordinary function cannot - the reference appears as a bare Lvar and the pass gives up. That is not special to increment. Its body is six nodes against a small_inline_size of five, and cross-module inlining is off, so the inliner cannot reach it. Writing the update directly does keep the unboxing, and is shorter than the call it replaces: Int.Ref.increment(v) -> v.contents = v.contents + 1 53 call sites across 30 test files change that way, and their generated JavaScript is byte-identical. Only two outputs move: Stdlib_Int loses an empty Ref object and its export, and test_incr_ref loses onExpression - added to pin that the primitive bound its argument before mentioning it twice, which has nothing left to test now that no expansion happens. Lambda.offset_ref and the Offset_ref builtin go with them. Nothing in lambda.ml now builds a term outside the constructors and the traversals. Int.Ref.t went too. It was a type alias for ref<int> introduced alongside the two functions, and with them gone the module held nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8g8qwBARAcvW9MyuKQq8H
cristianoc
added a commit
that referenced
this pull request
Sep 5, 2026
None of these were designed for ReScript. %incr, %decr and %refget arrived with OCaml's Pervasives in the 2016 initial export and were carried unexamined through every stdlib reshuffle since. Int.Ref itself was created in April 2025 (#7371) not because anyone wanted it, but as somewhere for the Pervasives.incr deprecation to point; the primitives it wrapped were removed for v13 two weeks ago. Outside this repository, GitHub code search finds no user of either the externals or the API. What the primitive bought was unboxing: expanding at the call site kept the field write syntactically visible, so Lam_pass_eliminate_ref could still turn a local ref into a mutable variable. A call through an ordinary function cannot - the reference appears as a bare Lvar and the pass gives up. That is not special to increment. Its body is six nodes against a small_inline_size of five, and cross-module inlining is off, so the inliner cannot reach it. Writing the update directly does keep the unboxing, and is shorter than the call it replaces: Int.Ref.increment(v) -> v.contents = v.contents + 1 53 call sites across 30 test files change that way, and their generated JavaScript is byte-identical. Only two outputs move: Stdlib_Int loses an empty Ref object and its export, and test_incr_ref loses onExpression - added to pin that the primitive bound its argument before mentioning it twice, which has nothing left to test now that no expansion happens. Lambda.offset_ref and the Offset_ref builtin go with them. Nothing in lambda.ml now builds a term outside the constructors and the traversals. Int.Ref.t went too. It was a type alias for ref<int> introduced alongside the two functions, and with them gone the module held nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8g8qwBARAcvW9MyuKQq8H
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stdlib.PairandStdlib.Int.Refto cover some functionality currently inPervasives.Pervasives.Pervasives_mini.