-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Minor clean-ups in runtime/io.c and runtime/caml/io.h #10136
Merged
Merged
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9656815
Remove the "for Cash only" fields `revealed` and `old_revealed`
xavierleroy c2f42c7
Refactor caml_ml_close_channel
xavierleroy bf8e44c
Use `Tag_cons` from `<caml/mlvalues.h>`
xavierleroy f7e32a3
Turn caml_getch and caml_putch into functions
xavierleroy aeb785e
In out_channels_list, report channels opened from OCaml only
xavierleroy 6021888
Remove misleading comment
xavierleroy 07c3295
Document `refcount` field better
xavierleroy 691655c
Changes entry for #10136
xavierleroy File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refcount
can go too. It is only increased insidecaml_alloc_channel
to prevent a race with finalisation (#8191), but finalisation no longer happens during allocation from C.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon closer inspection
refcount
needs to stay, but the comment/* prevent finalization during next alloc */
should be fixed to better match the purpose of the reference count :)The test for
refcount > 0
insidecaml_close_channel
is now pointless and can be changed into an assertionrefcount == 0
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would document
and remove
CHANNEL_FLAG_MANAGED_BY_GC
.