[PATCH] D10725: Improve testing for the C API

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Purely stylistic stuff, only super important point is the one on the test itself.

I'm happy for Eric to finish the code style review, but I think we should get the C code here to at least be consistent with the rest of llvm-c-test, and ideally as consistent as possible with LLVM's general coding conventions.

Feel free to land whenever Eric is happy.


================
Comment at: test/Bindings/llvm-c/echo.ll:14
@@ +13,3 @@
+  %6 = alloca ppc_fp128
+; TODO: label ?
+  %7 = alloca i7
----------------
The location of the TODOs seems odd to me. Maybe collect them at the top?

================
Comment at: tools/llvm-c-test/echo.cpp:174-178
@@ +173,7 @@
+      int OpCount = LLVMGetNumOperands(Src);
+      Dst = (OpCount == 0)
+        ? LLVMBuildRetVoid(Builder)
+        : LLVMBuildRet(
+            Builder,
+            clone_instruction(LLVMGetOperand(Src, 0), Builder, VMap));
+      break;
----------------
This seems a particularly awkward conditional expression... Maybe just use ifs?

================
Comment at: tools/llvm-c-test/echo.cpp:285
@@ +284,3 @@
+  LLVMBasicBlockRef Next = NULL;
+  while(TRUE) {
+    clone_bb(Cur, Dst, VMap);
----------------
You're relying on C99 elsewhere, please just pull ind <stdbool.h> and use normal bool syntax?


http://reviews.llvm.org/D10725