[PATCH] D98851: [OCaml] Add (get/set)_module_identifer functions

[Josh Berdine via Phabricator via llvm-commits]

jberdine added inline comments.


================
Comment at: llvm/bindings/ocaml/llvm/llvm_ocaml.c:342
+  const char *Name = LLVMGetModuleIdentifier(M, &Len);
+  return cstr_to_string(Name, (mlsize_t)Len);
+}
----------------
vaivaswatha wrote:
> jberdine wrote:
> > Does this still warn without the cast? I was expecting this cast to be not needed anymore.
> I didn't get a warning on my system. I added this to make the conversion explicit, because `size_t` can be strictly larger than `mlsize_t` which is defined to be `unsigned long`. I'm not sure if other systems might throw a warning though.
Ok, that's fine. Just note that the rest of this code is not following that convention currently. See e.g. llvm_get_string_attr_value which passes an unsigned Length to caml_alloc_string. Also, caml_string_length returns an mlsize_t which is implicitly converted to size_t in a number of places.


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

https://reviews.llvm.org/D98851