Skip to content

Remaining functions requiring explicit export#9924

Merged
dra27 merged 1 commit intoocaml:trunkfrom
dra27:explicit-export-PR
Sep 21, 2020
Merged

Remaining functions requiring explicit export#9924
dra27 merged 1 commit intoocaml:trunkfrom
dra27:explicit-export-PR

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Sep 17, 2020

Remaining functions requiring export control macros to re-enable Cygwin64 (NB all header changes are inside CAML_INTERNALS blocks)

@xavierleroy
Copy link
Copy Markdown
Contributor

Could you please explain the criteria you used to determine where CAMLextern is needed? For example, I agree caml_setup_stack_overflow_detection must be exported because it's used from the systhreads stub code. But I don't see any use of caml_set_action_pending outside of the run-time system. Did you find a use in stub code somewhere in OPAM? Do you anticipate this function to be used in stub code?

@dra27 dra27 force-pushed the explicit-export-PR branch 3 times, most recently from a761be6 to 6e8a7f3 Compare September 18, 2020 11:08
@dra27 dra27 force-pushed the explicit-export-PR branch from 6e8a7f3 to 17cceab Compare September 18, 2020 11:11
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Sep 18, 2020

I thought all these were either critical (the build fails) or mentioned in the C API. I appear to have made a mistake with caml_set_action_pending and caml_process_pending_actions_with_root. I've removed those two. The others are either clearly referenced outside the runtime or are referred to in the manual as being available (the Windows string functions and caml_process_pending_signals_exn) or were inconsistently declared before (caml_read_directory).

@dra27 dra27 mentioned this pull request Sep 18, 2020
Copy link
Copy Markdown
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. I agree with the criteria you state. This looks good to go.

@dra27 dra27 merged commit fed86fb into ocaml:trunk Sep 21, 2020
@dra27 dra27 deleted the explicit-export-PR branch September 21, 2020 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants