[PATCH] D18727: Add support for attribute in the C API

[Jakob Bornecrantz via llvm-commits]

Wallbraker added a subscriber: Wallbraker.
Wallbraker added a comment.

You should put the unrelated commits in another one, they are good clean ups.

I mirror the same comments as in http://reviews.llvm.org/D18733 regarding not directly removing the old functions.

Looks good otherwise, good work!


================
Comment at: include/llvm-c/Core.h:1530
@@ -1529,3 +1542,3 @@
  */
-const char *LLVMGetAsString(LLVMValueRef c, size_t* out);
 
----------------
While I agree with the change, this is unrelated and should go into another commit.

================
Comment at: include/llvm-c/Core.h:2063
@@ -2062,3 +2092,3 @@
  */
-const char  *LLVMGetMDString(LLVMValueRef V, unsigned* Len);
 
----------------
Unrelated change, in its own commit it is okay.

================
Comment at: lib/IR/Core.cpp:928-931
@@ -927,6 +969,6 @@
 
-const char *LLVMGetAsString(LLVMValueRef C, size_t* Length) {
-  StringRef str = unwrap<ConstantDataSequential>(C)->getAsString();
-  *Length = str.size();
-  return str.data();
 }
----------------
Unrelated change, in its own commit it is okay.

================
Comment at: tools/llvm-c-test/echo.cpp:763-769
@@ -761,1 +762,9 @@
+
+    // Copy attributes
+    for (unsigned k = 0, e = LLVMGetEndAttrKind(); k < e; ++k) {
+      if (LLVMHasFunctionAttr(Cur, k))
+        LLVMAddFunctionAttr(F, k);
+      if (LLVMHasReturnAttr(Cur, k))
+        LLVMAddReturnAttr(F, k);
+    }
 
----------------
As I said in D18733 a get number and get KindID at index function would make this less brute force. Alternatively a way to get them all at the same time, instead of "at index" if that is not the way that LLVM does things.


http://reviews.llvm.org/D18727