[llvm-commits] fix the MSVC warning in include/llvm-c/Core.h

[Chandler Carruth]

On Mon, Jan 23, 2012 at 4:02 PM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Mon, Jan 23, 2012 at 3:19 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Mon, Jan 23, 2012 at 3:16 PM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>> > On Mon, Jan 23, 2012 at 3:04 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>> >>
>> >> On Mon, Jan 23, 2012 at 2:53 PM, Paul Robinson <pogo.work at gmail.com>
>> >> wrote:
>> >> > On Mon, Jan 23, 2012 at 1:45 PM, Kostya Serebryany <kcc at google.com>
>> >> > wrote:
>> >> >> There are 8 functions dealing with LLVMAttribute
>> >> >> in include/llvm-c/Core.h.
>> >> >> Do you suggest to add 8 more functions that will deal with uint64_t?
>> >> >> Like this?
>> >> >>
>> >> >> void LLVMAddFunctionAttr(LLVMValueRef Fn, LLVMAttribute PA);
>> >> >> void LLVMAddFunctionAttr64(LLVMValueRef Fn, uint64_t PA);
>> >> >>
>> >> >> --kcc
>> >> >
>> >> > With a more meaningful type name, e.g. LLVMAttribute64 or some such,
>> >> > yes;
>> >>
>> >> Given the constraints of the stable C API (binary compatibility)
>> >> there's no way we can make this forwards compatible, is there?
>> >> (passing a struct we could add elements to in the future - that would
>> >> break binary compat, yes?)
>> >
>> >
>> > You simply have to make the struct opaque, and only manipulated through
>> API
>> > calls. However that's pretty high cost.
>>
>> Yeah - I was starting to see that. Pity - so we just add a new
>> generation of API calls every time we reach the limit.
>>
>
> So, do we go with
> void LLVMAddFunctionAttribute64(LLVMValueRef Fn, uint64_t PA);
>
> or with something more opaque like
> void LLVMAddFunctionAttribute(LLVMValueRef Fn, void *PAPtr);
>

Typically you would go with something inbetween:

struct LLVMAttribute;
void LLVMAddFOOBARFunctionAttribute(LLVMValueRef Fn, LLVMAttribute
*Attributes);

Then provide Add and Remove variations for all of the attributes,
substituting the attribute name for FOOBAR.


I really don't know that this (very heavyweight approach) is the right way
to go. I'd rather someone who is maintaining the LLVM C-APIs chime in, it
may be that just upgrading the interfaces to 64-bit integers is "enough"
for now.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120123/0a51c343/attachment.html>