[PATCH] D10725: Improve testing for the C API

[Amaury SECHET via llvm-commits]

deadalnix added inline comments.

================
Comment at: tools/llvm-c-test/echo.cpp:270
@@ +269,3 @@
+  while(true) {
+    clone_value(Cur, Builder, VMap);
+    Next = LLVMGetNextInstruction(Cur);
----------------
echristo wrote:
> Should probably have a clone_instruction separated out from clone_value so that anything weird is handled?
Can we defers that to a subsequent diff ? Changing this in that one will end up in rebase hell on my side as I have prepared various extension on that diff.

================
Comment at: tools/llvm-c-test/echo.cpp:283
@@ +282,3 @@
+    if (Prev != Cur) {
+      fprintf(stderr, "Next.Previous instruction is not Current\n");
+      exit(-1);
----------------
echristo wrote:
> Can you make this an error that's clear about what is going on? Ditto with the rest of them in this function, clone_bbs, etc.
What wording would you suggest ? I have hard time coming up with something much better. This just check that when you do one step forward and one step backward, you end up where you came from.

================
Comment at: tools/llvm-c-test/echo.cpp:334-337
@@ +333,6 @@
+  unsigned Count = LLVMCountParams(Src);
+  if (Count != LLVMCountParams(Dst)) {
+    fprintf(stderr, "Parameter count mismatch\n");
+    exit(-1);
+  }
+
----------------
echristo wrote:
> Can you add a test verifying this is working to your test?
Well if the code ever go through this codepath, that means there is a bug in the C API. I'm not sure how I can trigger it.


http://reviews.llvm.org/D10725