[PATCH] D19181: Make sure we have a Add/Remove/Has function for various thing that can have attribute.

[Amaury SECHET via llvm-commits]

deadalnix added a comment.

So I marked the function that'd be messed up if passed LLVMAttribute in the future.

That'd be easy to rename the Instr family with a Call family functions. and make Argument explicit in the arguments ones (which would be a plus no matter what, the current names are bad). The only one that would suffer a bit is the LLVMAddFunctionAttr as they'd get a clunky name.

The amount of code breakage would be much greater, however, transition can be done faster (as the new API can be introduced right away, without need to wait of a deprecation cycle for LLVMAttribute).

My gut feeling is that the longer transition with less breakage is preferable. I had in mind, add this. Wait a release. Remove LLVMAttribute, so everybody has to move to LLVMGetAttrKindID at this point. Wait one more release, and start messing with attributes ids.

If we are going to duplicate the API, then returning LLVMAttribute from LLVMGetAttrKindID is not really necessary anymore, we can go straight to the desired end result, but obviously, all code using attributes needs to be changed client side, not just adding a call to retrieve the attribute id.

@echristo , @whitequark , what are your thoughts ?


================
Comment at: include/llvm-c/Core.h:1953
@@ -1952,3 +1952,3 @@
  */
-void LLVMAddFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA);
+void LLVMAddFunctionAttr(LLVMValueRef Fn, unsigned KindID);
 
----------------
LLVMAddFunctionAttr

================
Comment at: include/llvm-c/Core.h:2082
@@ -2059,3 +2081,3 @@
  */
-void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA);
+void LLVMAddAttribute(LLVMValueRef Arg, unsigned KindID);
 
----------------
LLVMAddAttribute

================
Comment at: include/llvm-c/Core.h:2089
@@ -2066,3 +2088,3 @@
  */
-void LLVMRemoveAttribute(LLVMValueRef Arg, LLVMAttribute PA);
+void LLVMRemoveAttribute(LLVMValueRef Arg, unsigned KindID);
 
----------------
LLVMRemoveAttribute

================
Comment at: include/llvm-c/Core.h:2553
@@ -2522,2 +2552,3 @@
 
-void LLVMAddInstrAttribute(LLVMValueRef Instr, unsigned index, LLVMAttribute);
+void LLVMAddInstrAttribute(LLVMValueRef Instr, unsigned index,
+                           unsigned KindID);
----------------
LLVMAddInstrAttribute

================
Comment at: include/llvm-c/Core.h:2555
@@ -2524,1 +2554,3 @@
+                           unsigned KindID);
 void LLVMRemoveInstrAttribute(LLVMValueRef Instr, unsigned index,
+                              unsigned KindID);
----------------
LLVMRemoveInstrAttribute


http://reviews.llvm.org/D19181