Precise non-backward change done on weak-pointers#515
Closed
bobot wants to merge 2 commits intoocaml:4.03from
Closed
Precise non-backward change done on weak-pointers#515bobot wants to merge 2 commits intoocaml:4.03from
bobot wants to merge 2 commits intoocaml:4.03from
Conversation
Changes
Outdated
| Runtime system: | ||
| - PR#7173 GPR#515: Add a public C API for weak arrays and ephemerons. | ||
| (François Bobot and Jacques-Henri Jourdan) | ||
| * GPR#22: The undocumented layout of weak arrays have been changed. |
byterun/caml/weak.h
Outdated
|
|
||
| CAMLextern mlsize_t caml_ephemeron_key_length(value eph); | ||
| /** return the number of key in the ephemeron. The valid key offset goes | ||
| from [0] to the predecessor of the returned value. */ |
Contributor
There was a problem hiding this comment.
Comments like this usually start with capital letters, no?
Maybe you should add "inclusive" after "value" (if that's what you mean).
byterun/caml/weak.h
Outdated
| caml_ephemeron_unset_key, this function does not prevent the | ||
| incremental GC from erasing the value in its current cycle. The | ||
| values [eph1] and [eph2] must be ephemerons and the offset between | ||
| [off1] and [off1+len] and between [off2] and [off2+offset] must be |
Contributor
There was a problem hiding this comment.
s/offset/offsets (on line 75)
6c467f6 to
4bf1a58
Compare
Contributor
Author
|
Except for the name used for the exceptions, all the current comments have been taken into account. |
Changes
Outdated
|
|
||
| Runtime system: | ||
| - PR#7173 GPR#515: Add a public C API for weak arrays and ephemerons. | ||
| (François Bobot and Jacques-Henri Jourdan, review by Mark Shinwell) |
Member
There was a problem hiding this comment.
I suspect you broke the careful sorting of the issue list for this and GPR#22 just to spite me :-)
Member
|
@bobot can you make a new GPR for the bug fix? It's too late to add so much code in 4.03. |
Contributor
Author
|
@damiendoligez Ok. Could I keep only the bug fix in this GPR and move all the new code to another GPR? Just because this one target 4.03. |
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
5 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.
Fix #7173. There is a discussion in mantis if it should go to 4.03. I'm not convinced, except for the two first commits. I can create a separate GPR for them.