[PATCH] D125183: Add opaque pointers to the llvm-c API

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm-c/Core.h:556
+ */
+LLVMBool LLVMContextSupportsTypedPointers(LLVMContextRef C);
+
----------------
I'm a bit uncertain about this API. The supportsTypedPointers() naming dates back to a time where we supported mixed typed and opaque pointers, so "supports opaque pointers" and "not supports typed pointers" were not equivalent. There's a couple of different possibilities here:

* `LLVMContextSupportsTypedPointers()` as proposed.
* `LLVMContextHasOpaquePointers()` the inverse, which mirrors `LLVMContextSetOpaquePointers()` a bit more nicely.
* Expose `LLVMPointerTypeIsOpaque()` or similar instead, which would work on a pointer type rather than context.

The latter is slightly less general (you can only use it if you already have a pointer type), but it's the one that we actually use most often in practice. We only use `supportsTypedPointers()` for automatic switching into opaque pointer mode during LL/BC parsing.

I'd personally lean towards exposing `LLVMPointerTypeIsOpaque()`.


================
Comment at: llvm/tools/llvm-c-test/echo.cpp:1034
+    LLVMTypeRef Ty;
+    Ty = TypeCloner(M).Clone(LLVMGlobalGetValueType(Cur));
+
----------------
nit: Combine declaration and assignment.


================
Comment at: llvm/tools/llvm-c-test/main.c:52
   fprintf(stderr, "    Read bitcode file from stdin - print it back out\n\n");
+  fprintf(stderr, "  * --echo -opaque-pointers\n");
+  fprintf(stderr, "    Read bitcode file from stdin - print it back out in "
----------------
Make it `--echo --opaque-pointers` for the sake of consistency?


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

https://reviews.llvm.org/D125183