[PATCH] D19181: Map Attribute in the C API.

[whitequark via llvm-commits]

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