[PATCH] D125183: Add opaque pointers to the llvm-c API
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 9 02:18:20 PDT 2022
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
More information about the llvm-commits
mailing list