[PATCH] D19181: Map Attribute in the C API.
whitequark via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 11 17:13:01 PDT 2016
whitequark added inline comments.
================
Comment at: include/llvm-c/Core.h:388-391
@@ +387,6 @@
+enum {
+ LLVMAttributeReturnIndex = 0U,
+ // ISO C restricts enumerator values to range of 'int'
+ // (4294967295 is too large)
+ // LLVMAttributeFunctionIndex = ~0U,
+ LLVMAttributeFunctionIndex = -1,
----------------
I agree that, since the C and C++ APIs don't share this enum anyway, the numbering should be rectified as @Wallbraker says.
================
Comment at: include/llvm-c/Core.h:531
@@ +530,3 @@
+ */
+const char *LLVMGetAttributeKindAsString(LLVMAttributeRef A, unsigned *Length);
+
----------------
Does this allocate? If yes, needs to be mentioned in the docstring I think (etc etc LLVMDisposeString etc etc)
================
Comment at: tools/llvm-c-test/echo.cpp:800
@@ +799,3 @@
+ for (unsigned k = 0, e = LLVMGetEndAttrKind(); k < e; ++k) {
+ if (LLVMGetAttributeAtIndex(Cur, i, k)) {
+ auto A = LLVMCreateAttribute(Ctx, k, 0);
----------------
No. This is an incredibly inefficient way to iterate through attributes. A consumer of the C API that attempts to read bitcode would have to do what, at least ~50 FFI calls per function *or* call site just to discover attributes? This will not do.
http://reviews.llvm.org/D19181
More information about the llvm-commits
mailing list