I recently reimplemented the Attributes class. It now hides the data representation inside of an opaque class. In the near future, we will be extending this class to encompass many other attributes. The changes pose one problem, however. The C-API still uses the old data representation for passing along the Attributes class. In particular, these two functions: LLVMAttribute LLVMGetFunctionAttr(LLVMValueRef Fn); LLVMAttribute LLVMGetAttribute(LLVMValueRef Arg); return the LLVMAttribute, which is an enum that happens to be the internal bit representation of the Attributes object. This is bad for several reasons, not the least of which is that it's completely inextensible (e.g., we don't have room for LLVMAddressSafety). I would like to either remove these functions or create a new type called LLVMAttributeRef that references an Attributes object. The controversy is that this breaks the rule that C-APIs cannot change. But I don't really see an alternative, since this representation is going to become invalid with the new changes. What are people's thoughts on this? -bw
Hi Bill, On 18/10/12 10:35, Bill Wendling wrote:> I recently reimplemented the Attributes class. It now hides the data representation inside of an opaque class. In the near future, we will be extending this class to encompass many other attributes. > > The changes pose one problem, however. The C-API still uses the old data representation for passing along the Attributes class. In particular, these two functions: > > LLVMAttribute LLVMGetFunctionAttr(LLVMValueRef Fn); > LLVMAttribute LLVMGetAttribute(LLVMValueRef Arg); > > return the LLVMAttribute, which is an enum that happens to be the internal bit representation of the Attributes object. This is bad for several reasons, not the least of which is that it's completely inextensible (e.g., we don't have room for LLVMAddressSafety). > > I would like to either remove these functions or create a new type called LLVMAttributeRef that references an Attributes object. The controversy is that this breaks the rule that C-APIs cannot change. But I don't really see an alternative, since this representation is going to become invalid with the new changes.how about having LLVMAttribute be an enum specific to the C-API, and only cover the traditional attributes, and add a thunking layer in the implementation of LLVMGetFunctionAttr and friends: the C-API code can eg switch on the LLVMAttribute enum value, and call the appropriate routines in your new attributes code, converting back and forth between the enum and attributes objects. That preserves backwards compatibility. Then also add new routines and a new type for getting access to the new attributes stuff. That way old code that uses the old routines will continue to work but won't get access to the exciting new attributes you are going to add, while newer code can use the new routines and type. Ciao, Duncan.> > What are people's thoughts on this? > > -bw > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
On Oct 18, 2012, at 2:32 AM, Duncan Sands <baldrick at free.fr> wrote:> Hi Bill, > > On 18/10/12 10:35, Bill Wendling wrote: >> I recently reimplemented the Attributes class. It now hides the data representation inside of an opaque class. In the near future, we will be extending this class to encompass many other attributes. >> >> The changes pose one problem, however. The C-API still uses the old data representation for passing along the Attributes class. In particular, these two functions: >> >> LLVMAttribute LLVMGetFunctionAttr(LLVMValueRef Fn); >> LLVMAttribute LLVMGetAttribute(LLVMValueRef Arg); >> >> return the LLVMAttribute, which is an enum that happens to be the internal bit representation of the Attributes object. This is bad for several reasons, not the least of which is that it's completely inextensible (e.g., we don't have room for LLVMAddressSafety). >> >> I would like to either remove these functions or create a new type called LLVMAttributeRef that references an Attributes object. The controversy is that this breaks the rule that C-APIs cannot change. But I don't really see an alternative, since this representation is going to become invalid with the new changes. > > how about having LLVMAttribute be an enum specific to the C-API, and only cover > the traditional attributes, and add a thunking layer in the implementation of > LLVMGetFunctionAttr and friends: the C-API code can eg switch on the > LLVMAttribute enum value, and call the appropriate routines in your new > attributes code, converting back and forth between the enum and attributes > objects. That preserves backwards compatibility. Then also add new routines > and a new type for getting access to the new attributes stuff. That way old > code that uses the old routines will continue to work but won't get access to > the exciting new attributes you are going to add, while newer code can use > the new routines and type. >Hi Duncan, I'm not sure how we can use the enum values to switch back and forth since there is some overlap between the new Attributes::AttrVal values and the current LLVMAttribute values. So there could be some confusion. Do you have an idea of how to solve that? Maybe have different names for the new attribute enums or something? My preference would be to remove the above two functions entirely. We can replace them with equivalent functions that provide the same functionality but don't expose the internals of the Attributes class. -bw