[PATCH] D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5

[Alan via Phabricator via llvm-commits]

alan added inline comments.


================
Comment at: llvm/bindings/ocaml/llvm/llvm_ocaml.c:705
+  free(Temp);
+  CAMLreturn(Arr);
 }
----------------
alan wrote:
> Kakadu wrote:
> > This code looks suspicious. If we are going to return an array, why not to try to use `caml_alloc_array` function? 
> > 
> The original code uses `caml_alloc_tuple`, so that's what I did as well. I think `caml_alloc_array` (which takes a callback that converts the array elements to OCaml values) would work as well. Thanks for suggesting it.
I investigated more into [`caml_alloc_array`](https://github.com/ocaml/ocaml/blob/92adb0ff67dea9c14643a1c2eb6eeaf9d48629ef/runtime/alloc.c#L218) and found out that it expects the array to have a 0 sentinel value to mark the end rather than taking the length as an argument. This code can be adapted to use `caml_alloc_array` if it allocates `Length + 1` elements instead and sets the last element to `NULL`. Do the reviewers think this is a change I should make?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136400/new/

https://reviews.llvm.org/D136400