[PATCH] D134916: [llvm-ocaml] Add binding for constructing opaque pointers

[Alan via Phabricator via llvm-commits]

alan marked an inline comment as done.
alan added a comment.

It turns out that @nikic was correct to be fishy by pointing out that a "test" I added should not have passed! I figured out that I added the test function, but I had not added it to the list of tests at the bottom of the file, so it never ran. When I did run it, the test did indeed fail. I've updated the diff to fix this, as well as to add the missing word "space" in a doc comment that was pointed out.



================
Comment at: llvm/test/Bindings/OCaml/core.ml:47
+  insist (pointer_type_is_opaque (pointer_type_in_context context 0));
+  insist (not (pointer_type_is_opaque (pointer_type i8_type)))
 
----------------
nikic wrote:
> Hm, does this test really pass? It's not possible to mix opaque and typed pointers in the same context. The second call should be creating an opaque pointer as well.
It turns out that test should not have passed! It turned out I wasn't actually running the test I added because I neglected to add it to the list of tests at the bottom. When I added it, the test indeed failed. I've updated the patch to fix the test. Your expectation is right; both explicitly created opaque pointer and the pointer created from an i8 type are opaque.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134916