[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:1460
+/**
+ * Create an opaque pointer type in a context
+ *
----------------
nit: Missing period at end.


================
Comment at: llvm/tools/llvm-c-test/echo.cpp:1004
+    else
+      LLVMAddGlobal(M, TypeCloner(M).Clone(LLVMGlobalGetValueType(Cur)), Name);
 
----------------
You can use this unconditionally.


================
Comment at: llvm/tools/llvm-c-test/echo.cpp:1040
+    else
+      Ty = TypeCloner(M).Clone(LLVMGlobalGetValueType(Cur));
+
----------------
Same here.


================
Comment at: llvm/tools/llvm-c-test/echo.cpp:1402
 
-int llvm_echo(void) {
+static int do_llvm_echo(bool opaque_pointers) {
   LLVMEnablePrettyStackTrace();
----------------
Should be OpaquePointers for consistency?

Also, can we just change llvm_echo to accept the argument rather than adding two wrappers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125183