caml_result: an exception monad in C for resource-safe interfaces #13013

gasche · 2024-03-06T23:39:08Z

In the OCaml runtime, some C functions that in turn call OCaml code come in two variant:

the "raising' variant, for example caml_callback, will raise exceptions directly, as OCaml code would
the 'encoded exception' variant, for example caml_callback_exn, will not raise exceptions but encode them into the return value

The 'encoded exception' variant is important for resource-safe interfaces because the caller may want to run some cleanup/release code before propagating the exception, which the 'raising' variant cannot express -- C code cannot create OCaml exception handlers. On the other hand, the encoding of exceptions into the value type is a big hack that is unsafe in two different ways:

dynamically, the value encoding exceptions are not valid OCaml values, the GC will blow up if it encounters them; callers have to follow a strict discipline of checking for an exception right away and discarding the fake value
statically, reusing the value type means that the C compiler cannot check that we use these exceptions as intended, and will happily let us mix them with real values by mistake, making the dynamic unsafety harder to guard against

In #12407, @gadmm proposed to extend the approach of 'encoded exceptions' to more parts of the runtime, to make it possible to write resource-safe code in more places, but @xavierleroy pushed against the design due to the unsafety of encoded exceptions.
To be able to move forward, we need an approach for resource-safe APIs that is consensual; the present PR, which grew out of the discussios in #12407 and in particular discussions with @gadmm, is a proposed approach.

The PR proposes a new 'result' variant: caml_callback_result is similar to caml_callback_exn, but instead of returning an unsafe value it returns a new, distinct C type caml_result, that properly represents either a value or an exception. This is more safe:

dynamically, no invalid values are produced
statically, the type is incompatible with value, reminding us to deal with the exception in the caller

typedef struct {
  _Bool is_exception;
  value data;
} caml_result;

The present PR:

introoduces this caml_result type,
exposes _result variants for all functions that previously had an _exn variant, and
removes almost all uses of encoded exceptions in the runtime code itself.

For the _exn variants, the ones that were part of the public API are kept for compatibility, and those that were under CAML_INTERNALS are removed.

(The places where encoded exceptions are retained are the implementation of the caml_callback*_exn functions, which touch the bytecode interpreter and the assembly code in the runtime system. I preferred to leave them as-is, and wrap them inside caml_result on the outside.)

runtime/caml/mlvalues.h

MisterDA · 2024-03-10T22:24:12Z

runtime/caml/mlvalues.h

+   lets the caller implement proper cleanup and propagate the
+   exception themselves. */
+typedef struct {
+  _Bool is_exception;


Note that C99 _Bool cannot be used in public headers for C++ compatibility as C++ doesn't define it. We chose not to include stdbool.h in public headers out of fear of compatibility problems, so we don't have access to bool. The current consensus is to use int for public booleans. See #12787, #12702 for previous discussions.

Note that it is also possible to test for C++ and use bool in this case. (This was not possible with #12787.) In the long run C is headed towards deprecating _Bool in favour of bool.

It would be tempting to be able to use "_Bool or bool", because this plays nicely (nicer than int) with FFI based on code generation from the C headers. @gadmm tried Rust's bindgen, it creates a nice-looking Rust structure with a bool field.

However, in #12772 (comment) @MisterDA hinted at ABI issues coming from the fact that _Bool and bool are not guaranteed to have the same size. This could be a deal-breaker, right?

I am unfamiliar with these aspects of language interoperability -- it is interesting to have to deal with the absence of a type of booleans in 2024 -- so I welcome guidance. My intuition would be to give up on the nice type and use int (or maybe intnat?) to avoid any potential issues.

If we're going to use a C89 type it should probably be int, which is (still) the type of boolean expressions (such as (0 == 1), (a && b), or !p) in C23.

Use bool/_Bool or int; it should not make any change for Rust (nor to the caml_result type due to padding). I don't think ABI issues should be feared, as OCaml is unlikely to be supported on exotic platforms/compilers.

gasche · 2024-03-12T12:54:48Z

@gadmm told me (private communication) that he is fine with the overall approach if we can gather consensus around this.

damiendoligez

I like the idea and the code looks good modulo a few remarks, but don't merge before @xavierleroy has a look at it.

runtime/caml/callback.h

runtime/caml/mlvalues.h

runtime/signals.c

damiendoligez · 2024-03-13T15:09:21Z

runtime/signals.c

+CAMLexport caml_result caml_process_pending_actions_result(void)
 {
-  return caml_process_pending_actions_with_root_exn(Val_unit);
+  return caml_process_pending_actions_with_root_result(Val_unit);
 }


Too much wrapping going on here... I think this function is the same as caml_do_pending_actions_result.

This is not just caml_do_pending_actions_result, as there is a check on caml_check_pending_actions first. I inlined/streamlined the code -- and took the opportunity to write a small bit of documentation on the with_root variants. This new commit will need a review because it is less of a mechanical refactoring than the rest of the patch.

stedolan · 2024-03-13T18:06:48Z

This looks great, the encoded exception values returned by callback_exn are an easy trap to fall into.

(The places where encoded exceptions are retained are the implementation of the caml_callback*_exn functions, which touch the bytecode interpreter and the assembly code in the runtime system. I preferred to leave them as-is, and wrap them inside caml_result on the outside.)

This is the correct choice. The C ABI rules for returning a structure by value are tricky and vary significantly between platforms. It suits the C API (for which we have a compiler to implement those rules), but it would be unnecessarily complicated to follow these rules correctly in the per-platform assembly stubs.

gadmm

Overall I think it's a step in a good direction. I have some comments below.

Also, I think one important part remaining is to update the documentation (intf-c.etex), which should show that it is simpler indeed for living in harmony with the GC.

runtime/caml/mlvalues.h

runtime/callback.c

gadmm · 2024-03-15T18:04:08Z

runtime/callback.c

 }

+/* support code for caml_result */
+CAMLexport value caml_run_result(caml_result res)


I understand the allusion to runnable monads, but a name in the same vein as caml_raise_if_exception would be much less cryptic.

I considered the following naming choices:

caml_raise_if_exception is only in CAML_INTERNALS, so I could reuse that name and use caml_raise_if_encoded_exception for the old version, which is now only used in callback.c.

I could use consistent names for both versions, for example caml_raise_if_exception_{encoded,result}

or a new name altogether, for example caml_get_value_or_raise.

My current preference is (3) -- I like that the name is explicit that it can raise (this is better than caml_run_result), but also that it returns the value otherwise (this is better than caml_raise_if_exception).

I will move caml_raise_if_encoded_exception out of fail.h and into callback.c right away, and I think I will go for (3) as well, but I am interested in feedback on the name in any case.

gadmm · 2024-03-15T18:06:52Z

runtime/callback.c


+/* Result-returning variants of the above */
+
+CAMLexport caml_result caml_callbackN_result(


I am hoping to have _result variants for all public functions of the runtime at some point (hence for many private ones as well). Maybe some less noisy convention such as _res would turn out to be preferable in the long run?

I prefer to have a slightly longer but clearer name. _res could mean various things to me without context, for example resource or resolution.

Unless it is a codebase-wide convention, and it is already clear from the signature that it returns a caml_result? In addition, to me a complete word is more at risk of introducing ambiguity with words that precede: cf. run_callback_result in memprof.c.

gadmm · 2024-03-15T18:13:47Z

runtime/caml/mlvalues.h

+   lets the caller implement proper cleanup and propagate the
+   exception themselves. */
+typedef struct {
+  _Bool is_exception;


Note that it is also possible to test for C++ and use bool in this case. (This was not possible with #12787.) In the long run C is headed towards deprecating _Bool in favour of bool.

runtime/caml/mlvalues.h

runtime/caml/signals.h

runtime/compare.c

gadmm · 2024-03-15T21:40:35Z

runtime/fail_byt.c

+  {
+    caml_result result = caml_process_pending_actions_with_root_result(v);
+    v = result.data;
+  }


The blanket v = result.data; is very unusual so it could benefit from a comment saying that this is really intended.

It looks like one can do without the braces here.

New version.

caml_result result = caml_process_pending_actions_with_root_result(v); /* If the result is a value, we want to assign it to [v]. If the result is an exception, we want to continue raising it instead of [v]. The line below does both these things at once. */ v = result.data;

runtime/fail_nat.c

xavierleroy

Minor remarks below. Overall, this looks good, but as discussed privately with @gasche I'd be reassured if this design was tested on the OCaml/Rust interface, to make sure it works well for this particularly demanding library.

runtime/caml/mlvalues.h

gadmm · 2024-03-18T14:49:51Z

This needs to be rebased on top of the statmemprof patches, and memprof.c adapted to the new _res convention.

gasche · 2024-03-19T17:11:15Z

I rebased this PR on top of trunk, and took all the standing review comments into account. Thanks a lot for all this detailed feedback.

Notable changes:

@damiendoligez suggested to simplify caml_process_pending_actions_result, this is done in a commit called "document and refactor [caml_process_pending_actions_result" (I am not giving specify hashes as they will be invalidated on my next rebase, so I recommend to look for it in the list of commits.
as @gadmm pointed out, the newly merged memprof.c codebase used _exn functions quite a bit. I converted them to _result functions in "runtime: remove encoded exceptions from memprof.c", and this can be taken as an example of _result-heavy code. (@gadmm had some justified worries about the heaviness of the current naming conventions, maybe we can check that they are acceptable there)
@gadmm wanted another name for caml_run_result, closer to caml_raise_if_exception. I moved caml_raise_if_exception (which was CAML_INTERNALS) out of the way, and renamed caml_run_result into caml_get_value_or_raise -- but my mind is at peace with the idea that I may need to change it again and again. Instead of a function exposed in mlvalues.h and defined in fail_{byt,nat}.c, this is now an inline function of fail.h.

Besides the naming of caml_get_value_or_raise, there are some questions left in the air about whether we want the conversion functions between caml_result and encoded exceptions to be made public or not.

gasche · 2024-03-19T17:31:05Z

Of course I forgot two points.

I said that I would move from _Bool to int, but I am feeling nostalgic about a proper boolean type already -- and then maybe I need to use intnat, I am not sure. I am hoping that more knowledgeable people will conclusively say that one of these options is much better than the other.

@xavierleroy asked whether this caml_result design works in Rust. The answer is "yes, according to bindgen" (the standard Rust tool that takes C headers and generates compatible Rust definitions for them), as reported by @gadmm (quoting from his log):

Input: h.h

typedef unsigned long long value;

typedef struct caml_result {
    _Bool is_exception;
    value data;
} caml_result;

caml_result caml_process_pending_actions_res();

Output: bindgen h.h (with some boring tests elided)

/* automatically generated by rust-bindgen 0.69.4 */

pub type value = ::std::os::raw::c_ulonglong;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct caml_result {
    pub is_exception: bool,
    pub data: value,
}
#[test]
fn bindgen_test_layout_caml_result() {
    const UNINIT: ::std::mem::MaybeUninit<caml_result> = ::std::mem::MaybeUninit::uninit();
    let ptr = UNINIT.as_ptr();
    assert_eq!(
        ::std::mem::size_of::<caml_result>(),
        16usize,
        concat!("Size of: ", stringify!(caml_result))
    );
    assert_eq!(
        ::std::mem::align_of::<caml_result>(),
        8usize,
        concat!("Alignment of ", stringify!(caml_result))
    );
}
extern "C" {
    pub fn caml_process_pending_actions_res() -> caml_result;
}

This suggests that the caml_result type does have a representation compatible with Rust structures (when constrained by the #[repr(C)] pragma), so interoperability should work fine. This is not an idiomatic way to deal with results-or-failures in Rust (as a user I would expect a Result<OCamlValue,OCamlValue> type instead), but conversion functions to can be used on the Rust side if desired. We cannot hope to do better today as the standard Result type in Rust is not repr(C), so there is no way to bind it directly from C in a compatible way. (While studying this I found the tool cbindgen that supports repr(C) enums as really tagged unions, but it does not apply here.)

xavierleroy · 2024-03-19T17:42:31Z

I said that I would move from _Bool to int, but I am feeling nostalgic about a proper boolean type already -- and then maybe I need to use intnat

int should work, as we don't have to guarantee size and alignment for the caml_result type. I also thought of

typedef struct caml_result {
    enum { OK = 0, ERR = 1 } status;
    value data;
} caml_result;

which gets you even closer to the result type in Rust or OCaml. Plus, it leaves open the possibility of adding other kinds of results later, such as FATAL_ERROR = 2, i.e. a result that caml_get_value_or_raise would turn into a fatal error. Just an idea.

NickBarnes

Looks good to me.

NickBarnes · 2024-03-20T10:12:56Z

runtime/caml/mlvalues.h

+   lets the caller implement proper cleanup and propagate the
+   exception themselves. */
+typedef struct {
+  _Bool is_exception;


If we're going to use a C89 type it should probably be int, which is (still) the type of boolean expressions (such as (0 == 1), (a && b), or !p) in C23.

NickBarnes · 2024-03-20T17:45:16Z

runtime/callback.c

+CAMLexport caml_result caml_callback_result(
+  value closure, value arg)
+{
+  return Result_encoded(caml_callback_exn(closure, arg));


I suppose it would be more effort to make caml_callback_exn (etc) be a wrapper around caml_callback_result, rather than vice versa. But in future, since the "encoded" versions are frowned upon, I can imagine switching them over (and even, for that matter, pushing the change down into caml_callback_asm).

The plan is to keep using the "encoded exceptions" format for code that is defined in the assembly runtime, because there it is much easier to flip a boolean than to return a structure. Now assuming that all the caml_callback*_asm functions return an encoded exception, it is more logical I thnink to have the _exn version be primitive and the _result version defined on top of it -- otherwise the _exn version would go back and forth, from encoded exceptions to caml_result to encoded exception again.

NickBarnes · 2024-03-20T17:46:19Z

runtime/callback.c

 CAMLexport value caml_callback (value closure, value arg)
 {
-  return caml_raise_if_exception(caml_callback_exn(closure, arg));
+  return encoded_value_or_raise(caml_callback_exn(closure, arg));


or caml_get_value_or_raise(caml_callback_result(closure, arg)) ?

gadmm · 2024-03-20T17:52:50Z

runtime/callback.c


+/* Result-returning variants of the above */
+
+CAMLexport caml_result caml_callbackN_result(


Unless it is a codebase-wide convention, and it is already clear from the signature that it returns a caml_result? In addition, to me a complete word is more at risk of introducing ambiguity with words that precede: cf. run_callback_result in memprof.c.

runtime/caml/fail.h

gadmm · 2024-03-20T18:00:12Z

runtime/caml/memory.h

  }

+#define CAMLlocalresult(res) \
+  res.data = Val_unit; \


shouldn't this be

Suggested change

res.data = Val_unit; \

caml_result res = Result_unit; \

or am I missing something?

The CAMLlocalresult definition was wrong, and it was not used in the runtime. I believe that it is useful to have such a macro (it is tricky to write, and it is easy to write subtly wrong code by forgetting about it), so what I did was to (1) fix it and (2) tweak the code to use it somewhere (where it is not strictly necessary). As you have seen, #13061 is an offshoot of this work, and I may wait for that one to be resolved to decide what to do here.

I originally introduced CAMLlocalresult because I had a use for it in the runtime, but the previous use-case apparently was just in pre-PR experiments and never made it to the PR. It is notable that it is not strictly necessary in the runtime itself, where the cleanup code in the exception case does not require rooting, and the usage code in the value case is either (1) an immediate return or (2) a continuation assuming that the value is () and never using it again.

runtime/caml/mlvalues.h

gadmm · 2024-03-20T18:05:46Z

runtime/caml/mlvalues.h

+   lets the caller implement proper cleanup and propagate the
+   exception themselves. */
+typedef struct {
+  _Bool is_exception;


Use bool/_Bool or int; it should not make any change for Rust (nor to the caml_result type due to padding). I don't think ABI issues should be feared, as OCaml is unlikely to be supported on exotic platforms/compilers.

runtime/caml/mlvalues.h

runtime/memprof.c

gadmm · 2024-03-20T18:37:29Z

runtime/signals.c

+CAMLexport caml_result caml_process_pending_actions_result(void)
 {
-  return caml_process_pending_actions_with_root_exn(Val_unit);
+  if (caml_check_pending_actions()) {


I don't see what's wrong with return caml_process_pending_actions_with_root_result(Val_unit);. It's better to not duplicate the code in this case.

gadmm · 2024-03-20T18:42:18Z

@gasche has already explored the idea of having the result type extensible in the future. However I don't see how it fits with the stability guarantees needed.

I don't think there is a decisive argument for/against any of the suggested representations, just pick any one.

In the context of ocaml#13013, I would like to be able to use CAMLxparam1 on a lvalue that is not a variable: CAMLxparam1(result.data); This is not currently supported, as the definition of CAMLxparam1 uses its argument to build a variable name: #define CAMLxparam1(x) \ struct caml__roots_block caml__roots_##x; \ CAMLunused_start int caml__dummy_##x = ( \ (caml__roots_##x.next = *caml_local_roots_ptr), \ (*caml_local_roots_ptr = &caml__roots_##x), \ (caml__roots_##x.nitems = 1), \ (caml__roots_##x.ntables = 1), \ (caml__roots_##x.tables [0] = &x), \ 0) \ CAMLunused_end Let us break down this definition: 1. What the macro does is to add a new element to a singly-linked list stored in `*caml_local_roots_ptr`. 2. This element has type `struct caml__root_block`, and the structure is initialized by first declaring it and then initializing its field. We use the macro argument `x` to derive the structure name, `caml__roots_##x`. 3. We want to initialize `caml__roots_##x` and also assign `*caml_local_roots_ptr`, but instead of a standard `do { ... } while (0)` block we place these assignments inside the declaration of an unused variable `caml__dummy_##x`. This is done to respect the C89 rule that all declarations must be grouped together at the beginning -- putting a statement here would break this property for declarations coming right after the macro invocation. Now that we use C99, we can use a structure initializer to build the structure value without first declaring a variable of this type, and we don't need to group all definitions at the beginning so our macro can have arbitrary statements. The present commit changes the definition of `CAMLxparam1` to become: #define CAMLxparam1(x) \ *caml_local_roots_ptr = &(struct caml__roots_block){ \ .next = *caml_local_roots_ptr, \ .nitems = 1, \ .ntables = 1, \ .tables = { &(x), } } which supports `CAMLxparam1(result.data)` just fine.

gasche · 2024-04-01T19:34:46Z

I did a round of modification following the reviews, thanks!

For now I have used int for the is_exception field.

I also thought of
typedef struct caml_result {
    enum { OK = 0, ERR = 1 } status;
    value data;
} caml_result;
which gets you even closer to the result type in Rust or OCaml. Plus, it leaves open the possibility of adding other kinds of results later, such as FATAL_ERROR = 2, i.e. a result that caml_get_value_or_raise would turn into a fatal error. Just an idea.

I did, in a previous iteration of this PR, use an enum with two values: CAML_VALUE and CAML_EXCEPTION (I am not sure if enum names need namespacing in C, but I thought that they did). The result was fairly heavy/cumbersome, because I would then feel tempted to use switch on this field, and have to handle the default case, etc. Maybe I should revisit this with explicit equality tests instead of a switch.

The only form of extensibility that I could think of was an eventual future version of caml_result that could also represent unhandled effects. But this seemed like science-fiction, I doubted that the extension would be grateful, and I am skeptical that we would ever try to have C layers support continuation capture. I don't know about Fatal_error, but a closely related idea is Asynchronous_exception if the proposal to distinguish them from "normal exception" goes through.

What I would ideally like to do to have something more future-proof than the current definition is the following:

Expose a function to test if a result is an exception, instead of accessing the field directly. But I don't know how to name this function, in a way that would not conflict with Is_exception_result, which was an excellent name but is now taken for encoded exceptions. (I thought of reappropriating the name by removing the previous function, but that sounds a bit wild for something outside CAML_INTERNALS.)
Hide the definition of caml_result as a structure, to force users to go through the Result_value and Result_exception constructors, and I suppose some accessors as well. But I don't know how to do this in C in a way that does not hurt performance by breaking the inlining of the trivial wrappers.

gasche · 2024-04-01T20:48:12Z

Also: @gadmm proposed changing the convention from foo_result to foo_res. His arguments make sense, so I am thinking of giving the rename a try to see how it goes in the code, but I haven't had the time to do so yet.

gadmm · 2024-04-01T21:09:54Z

Thanks for the feedback, I was going to insist about it :) In the related PR, the _result suffix often leads to ambiguities

caml_threadstatus_new_exn -> caml_threadstatus_new_result
caml_thread_domain_initialize_hook_exn -> caml_thread_domain_initialize_hook_result
caml_check_error_exn -> caml_check_error_result

(statmemprof still uses _exn internally, to be converted next)

Suggested-by: Damien Doligez <damien.doligez@inria.fr>

Suggested-by: Guillaume Munch-Maccagnoni <Guillaume.Munch-Maccagnoni@inria.fr>

gasche · 2024-06-04T21:42:34Z

I propose documentation for caml_result in the FFI chapter of the manual in #13216.

gasche · 2024-07-15T14:48:11Z

Note to self: the Coq implementation uses _exn functions in its fork of the bytecode interpreter:

# /usr/bin/ld: kernel/byterun/libcoqrun_stubs.a(coq_interp.o): in function `coq_interprete':
# /home/gasche/Prog/ocaml/github-trunk/_opam/.opam-switch/build/coq-core.8.19.2/_build/default/kernel/byterun/coq_interp.c:582: undefined reference to `caml_process_pending_actions_exn'

To fix this, Coq would need conditional compilations depending on the OCaml version, or we would have to restore caml_process_pending_actions_exn for backwards-compatibility.

gadmm · 2024-07-15T15:08:03Z

This is part of the public interface indeed. In fact, I checked that you preserved the functions part of the public interface so it did not raise alarm: caml_process_pending_actions_exn is still declared in signals.h indeed. It seems right to implement caml_process_pending_actions_exn in terms of caml_process_pending_actions_res for backwards-compatibility.

I made a mistake when introducing caml_process_pending_actions_res (ocaml#13013), currently in trunk `caml_process_pending_actions_exn` is exported in signals.h but its implementation is removed. This function is not CAML_INTERNALS, and Coq uses it. To restore this function I reintroduced a helper function to transform `caml_result` values back into encoded exception (earlier ocaml#13013 designs had this). It is named `caml_result_get_encoded_exception`, in fail.h, CAML_INTERNALS-only.

gasche · 2024-07-16T09:06:57Z

I restore caml_process_pending_actions_exn in #13304.

I made a mistake when introducing caml_process_pending_actions_res (ocaml#13013), currently in trunk `caml_process_pending_actions_exn` is exported in signals.h but its implementation is removed. This function is not CAML_INTERNALS, and Coq uses it. To restore this function I reintroduced a helper function to transform `caml_result` values back into encoded exception (earlier ocaml#13013 designs had this). It is named `caml_result_get_encoded_exception`, in fail.h, CAML_INTERNALS-only.

gasche mentioned this pull request Mar 6, 2024

Resource-safe C interface: caml/fail.h #12407

Merged

ghost reviewed Mar 7, 2024

View reviewed changes

runtime/caml/mlvalues.h Outdated Show resolved Hide resolved

gasche force-pushed the caml_result branch 3 times, most recently from dbfff01 to 4e259db Compare March 7, 2024 22:15

MisterDA reviewed Mar 10, 2024

View reviewed changes

damiendoligez approved these changes Mar 13, 2024

View reviewed changes

gadmm reviewed Mar 15, 2024

View reviewed changes

xavierleroy reviewed Mar 18, 2024

View reviewed changes

runtime/caml/mlvalues.h Outdated Show resolved Hide resolved

runtime/caml/mlvalues.h Outdated Show resolved Hide resolved

runtime/caml/mlvalues.h Outdated Show resolved Hide resolved

gasche force-pushed the caml_result branch from 4e259db to 5b9070e Compare March 18, 2024 21:43

gasche mentioned this pull request Mar 19, 2024

Statmemprof resurrected #12923

Merged

gasche force-pushed the caml_result branch from 5b9070e to 1e4cab7 Compare March 19, 2024 16:28

NickBarnes approved these changes Mar 20, 2024

View reviewed changes

gadmm reviewed Mar 20, 2024

View reviewed changes

gasche mentioned this pull request Mar 30, 2024

use C99 features to support arbitrary lvalues in CAMLxparam* macros #13061

Closed

gasche force-pushed the caml_result branch 2 times, most recently from 2cbf39a to 483e1f8 Compare April 1, 2024 19:28

gadmm mentioned this pull request Apr 1, 2024

Correctly handle errors during Domain.spawn #12410

Merged

gasche force-pushed the caml_result branch from 483e1f8 to 0f5ebbf Compare May 19, 2024 16:22

gasche added 14 commits June 4, 2024 16:49

use caml_result in domain.c

16400d6

use caml_callback_result when the exception is ignored

4a51f7e

convert all all remaining users of _exn variants in the runtime

735bac0

(statmemprof still uses _exn internally, to be converted next)

runtime: remove encoded exceptions from memprof.c

f04e109

document and refactor [caml_process_pending_actions_result]

ad049e5

Suggested-by: Damien Doligez <damien.doligez@inria.fr>

mlvalues.h: move encoded exceptions to the end to discourage usage

4602cd9

Suggested-by: Guillaume Munch-Maccagnoni <Guillaume.Munch-Maccagnoni@inria.fr>

runtime: make caml_raise_if_exception private to callback.c

6d80a16

runtime: rename caml_run_result in caml_get_value_or_raise

8df88ca

Changes

65156c6

review: remove Encoded_result, move Result_encoded to callback.c

d656388

review: make is_exception 'int' to avoid _Bool compatibility issues

d7a1370

gc_full_major: make it clearer that no rooting is necessary

2b40cfd

review: make caml_get_value_or_raise non-internal

e293f7d

review: fix CAMLlocalresult and use it to ensure that it works

e9c6a5d

gasche force-pushed the caml_result branch from 92533a5 to ddeab89 Compare June 4, 2024 14:54

gasche added 3 commits June 4, 2024 17:02

mlvalues.h: expose caml_result as an more abstract type

8ad16ee

caml_result: avoid using the 'private' fields directly when possible

a63eb68

runtime: rename all [foo_result] function into [foo_res]

5f621b0

Suggested-by: Guillaume Munch-Maccagnoni <Guillaume.Munch-Maccagnoni@inria.fr>

gasche force-pushed the caml_result branch from ddeab89 to 5f621b0 Compare June 4, 2024 15:02

gasche added the merge-me label Jun 4, 2024

gasche mentioned this pull request Jun 4, 2024

Document caml_result in the manual (FFI chapter) #13216

Merged

gasche merged commit 272cf5d into ocaml:trunk Jun 5, 2024

gasche mentioned this pull request Jul 16, 2024

restore caml_process_pending_actions_exn #13304

Merged

gasche mentioned this pull request Jul 20, 2024

Complete caml_startup_*_exn functions with caml_startup_*_res variants #13324

Open


		/* Result-returning variants of the above */

		CAMLexport caml_result caml_callbackN_result(

caml_result: an exception monad in C for resource-safe interfaces #13013

caml_result: an exception monad in C for resource-safe interfaces #13013

Uh oh!

Conversation

gasche commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Mar 12, 2024

Uh oh!

damiendoligez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stedolan commented Mar 13, 2024

Uh oh!

gadmm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xavierleroy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gadmm commented Mar 18, 2024

Uh oh!

gasche commented Mar 19, 2024

Uh oh!

gasche commented Mar 19, 2024

Uh oh!

xavierleroy commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NickBarnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gasche commented Mar 6, 2024 •

edited

Loading

xavierleroy commented Mar 19, 2024 •

edited

Loading

gadmm commented Apr 1, 2024 •

edited

Loading