[PATCH] D18729: [llvm-c] Improve IR Introspection: Add ValueKind

[Amaury SECHET via llvm-commits]

deadalnix added a subscriber: deadalnix.
deadalnix added a comment.

Could you use arc next time to submit a patch ? It is very hard to see if there is missing checks in the test.

Overall, if the stability of these values cannot be guaranteed then that is an issue and a mechanism to retrieve the ID, either from a  string or from a C enum with guaranteed values.

More generally, I'm not sure how much value does this bring to the table as it seems fairly redundant with the LLVMIsAXXXX mechanism. It can be argued that it create a few extra tests, but using the C API means wrapping and unwrapping all the time, and doing these check in the process, so I'm not sure it is worth it.


================
Comment at: include/llvm-c/Core.h:252
@@ +251,3 @@
+#define HANDLE_VALUE(Name) LLVM##Name##ValueKind,
+#include "llvm/IR/Value.def"
+} LLVMValueKind;
----------------
Are these value stable ?

================
Comment at: tools/llvm-c-test/echo.cpp:295
@@ -294,3 @@
-  if (!LLVMIsAConstantExpr(Cst))
-    report_fatal_error("Expected a constant expression");
-
----------------
ubsan wrote:
> whitequark wrote:
> > Why did you remove this?
> The exact same test is also at the top of the function.
No it is not.


http://reviews.llvm.org/D18729