Merged
Conversation
We are trying to avoid using the 'lazy' machinery in the part of the compiler distribution that is used during bootstrap (to build ocamlc.byte). This avoids depending on the arguably complex runtime support for lazy, and it makes it easier to change this runtime support. There was a use of `lazy` in typing/ctype.ml which does not appear to require any sharing/memoization; we replace it by `unit -> 'a` thunks.
Contributor
|
Might it be useful to have a CI test that |
Member
Author
|
I am not sure that the extra complexity/ceremony is worth it: we will have false alarms for people that use |
Octachron
approved these changes
Dec 5, 2023
Member
Octachron
left a comment
There was a problem hiding this comment.
The delayed_copies reference is never published outside of copy_sep, and it is traversed only once before leaving the copy_sep function. That seems obviously correct to me?
Member
Author
|
The AppVeyor CI failed due to the usual spurious failure in memory-model/forbidden ( #12425 ). Merging. |
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.
We are trying to avoid using the 'lazy' machinery in the part of the compiler distribution that is used during bootstrap (to build ocamlc.byte). This avoids depending on the arguably complex runtime support for lazy, and it makes it easier to change this runtime support.
As discussed with @lthls yesterday, there was a use of
lazyin typing/ctype.ml which does not appear to require any sharing/memoization; we replace it byunit -> 'athunks. (I find it a bit surprising that we would uselazywhenunit -> 'asuffices, so I would be reassured by a confirmation by @garrigue that the change is indeed correct.)