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

Add caml_startup_exn (was MPR#385) #953

Merged
merged 1 commit into from Feb 21, 2017

Conversation

Projects
None yet
4 participants
@mshinwell
Contributor

mshinwell commented Dec 8, 2016

This pull request implements an old request for a caml_startup_exn function that can be used to catch exceptions coming from top level module initialisers. It seemed straightforward enough as to be worth doing.

@mshinwell mshinwell added this to the 4.05.0 milestone Dec 8, 2016

@mshinwell mshinwell requested a review from damiendoligez Dec 8, 2016

Show outdated Hide outdated manual/manual/cmds/intf-c.etex
The "caml_startup" function calls the uncaught exception handler (or
enters the debugger, if running under ocamldebug) if an exception escapes
from a top-level module initialiser. Such exceptions may be caught in the
C code by instead using the "caml_startup" function and testing the result

This comment has been minimized.

@gasche

gasche Dec 8, 2016

Member

caml_startup_exn

@gasche

gasche Dec 8, 2016

Member

caml_startup_exn

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell
Contributor

mshinwell commented Dec 27, 2016

@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Feb 21, 2017

Contributor

Can you please rebase on trunk to get rid of the silly conflict on Changes? Then please merge in trunk and cherry-pick to 4.05, or ask me to do so. Thanks!

Contributor

xavierleroy commented Feb 21, 2017

Can you please rebase on trunk to get rid of the silly conflict on Changes? Then please merge in trunk and cherry-pick to 4.05, or ask me to do so. Thanks!

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Feb 21, 2017

Contributor

I will merge this once the Travis checks etc. have completed.
Just to check: this should be on 4.05 even though it's not a bug fix?

Contributor

mshinwell commented Feb 21, 2017

I will merge this once the Travis checks etc. have completed.
Just to check: this should be on 4.05 even though it's not a bug fix?

@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Feb 21, 2017

Contributor

this should be on 4.05 even though it's not a bug fix?

As far as I understand: only bugfixes go on 4.04.1, but low-risk, backward-compatible, uncontroversial additions can still go to 4.05. We'll check with @damiendoligez .

Contributor

xavierleroy commented Feb 21, 2017

this should be on 4.05 even though it's not a bug fix?

As far as I understand: only bugfixes go on 4.04.1, but low-risk, backward-compatible, uncontroversial additions can still go to 4.05. We'll check with @damiendoligez .

@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Feb 21, 2017

Contributor

Also: could you please rebase your branch on trunk rather than merge trunk in your branch? The latter produces ugly merge commits in the trunk history. The former is a lot cleaner.

Contributor

xavierleroy commented Feb 21, 2017

Also: could you please rebase your branch on trunk rather than merge trunk in your branch? The latter produces ugly merge commits in the trunk history. The former is a lot cleaner.

@xavierleroy xavierleroy merged commit 90668e9 into ocaml:trunk Feb 21, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

xavierleroy added a commit that referenced this pull request Feb 21, 2017

Merge pull request #953 from mshinwell/caml_startup_exn
MPR#386: Add caml_startup_exn
@xavierleroy

This comment has been minimized.

Show comment
Hide comment
@xavierleroy

xavierleroy Feb 21, 2017

Contributor

Merged on trunk and cherry-picked on 4.05 (with @damiendoligez 's blessing).

Contributor

xavierleroy commented Feb 21, 2017

Merged on trunk and cherry-picked on 4.05 (with @damiendoligez 's blessing).

@@ -501,6 +501,13 @@ let link_bytecode_as_c ppf tolink outfile =
\n caml_sections, sizeof(caml_sections),\
\n argv);\
\n}\
\nvalue caml_startup_exn(char ** argv)\

This comment has been minimized.

@murmour

murmour Feb 22, 2017

Contributor

caml_startup_exn should have been declared with CAMLextern above (see line 463).
I've fixed this in #71 this way:

-\nCAMLextern void caml_startup_code(\
-\n           code_t code, asize_t code_size,\
-\n           char *data, asize_t data_size,\
-\n           char *section_table, asize_t section_table_size,\
-\n           char **argv);\n";
+\n#include <caml/startup.h>\n";
@murmour

murmour Feb 22, 2017

Contributor

caml_startup_exn should have been declared with CAMLextern above (see line 463).
I've fixed this in #71 this way:

-\nCAMLextern void caml_startup_code(\
-\n           code_t code, asize_t code_size,\
-\n           char *data, asize_t data_size,\
-\n           char *section_table, asize_t section_table_size,\
-\n           char **argv);\n";
+\n#include <caml/startup.h>\n";

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment