[PATCH] D10725: Improve testing for the C API

Amaury SECHET via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 17:27:55 PST 2016


deadalnix added inline comments.

================
Comment at: test/Bindings/llvm-c/echo.ll:15
@@ +14,3 @@
+  %6 = alloca ppc_fp128
+  %7 = alloca i7
+  %8 = alloca void (i1)*
----------------
Done

================
Comment at: tools/llvm-c-test/echo.cpp:21
@@ +20,3 @@
+#include "llvm/ADT/DenseMap.h"
+
+#include <stdio.h>
----------------
Done

================
Comment at: tools/llvm-c-test/echo.cpp:33
@@ +32,3 @@
+// Because the C API uses opaques pointer types, their alignement is unknown.
+// As a result, we need to roll out our own implementation.
+template<typename T>
----------------
Done

================
Comment at: tools/llvm-c-test/echo.cpp:45
@@ +44,3 @@
+    }
+    static unsigned getHashValue(const T *PtrVal) {
+      return hash_value(PtrVal);
----------------
Done

================
Comment at: tools/llvm-c-test/echo.cpp:81
@@ +80,3 @@
+      if (ParamCount > 0) {
+        Params = (LLVMTypeRef*) malloc(ParamCount * sizeof(LLVMTypeRef));
+        LLVMGetParamTypes(Src, Params);
----------------
OK This is not really working for that one (The API expect to dump in a buffer, and the C++y code is worse than straight C) but I did it for call instructions.

================
Comment at: tools/llvm-c-test/echo.cpp:149
@@ +148,3 @@
+
+static LLVMValueRef clone_value(LLVMValueRef Src, LLVMBuilderRef Builder, ValueMap &VMap) {
+  const char *Name = LLVMGetValueName(Src);
----------------
I think naming the function correctly makes it clear. Also added some comment withing the function to explain what is going on.

================
Comment at: tools/llvm-c-test/echo.cpp:179
@@ +178,3 @@
+  if (LLVMIsUndef(Src)) {
+    LLVMContextRef Ctx = LLVMGetModuleContext(get_module(Builder));
+    LLVMTypeRef Ty = clone_type(LLVMTypeOf(Src), Ctx);
----------------
Just used ifs :)

================
Comment at: tools/llvm-c-test/echo.cpp:287
@@ +286,3 @@
+      exit(-1);
+    }
+
----------------
Used true/false and nullptr all over.

================
Comment at: tools/llvm-c-test/echo.cpp:303
@@ +302,3 @@
+  LLVMBasicBlockRef Last = LLVMGetLastBasicBlock(Src);
+
+  LLVMBasicBlockRef Cur = First;
----------------
Removed braces around one liner.


http://reviews.llvm.org/D10725





More information about the llvm-commits mailing list