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

[Nick Lewycky]

Kostya Serebryany wrote:
>
>
> On Tue, Jan 24, 2012 at 11:47 PM, Chandler Carruth <chandlerc at google.com
> <mailto:chandlerc at google.com>> wrote:
>
>     FYI, I think Nick wanted to chime in on this, but he's kinda busy,
>     so I just wanted to update thread and put it on his radar.

Aha!! I was looking for this thread! :)

I think we should stop worrying about the C ABI. The first piece of C 
API was the libLTO library which was designed at a high-level to 
accomplish something interesting using the C++ APIs, but was insulated 
from the details of LLVM underneath. Similarly with libClang on the 
clang side.

Then there's the rest of the C API stuff, which is literally just a thin 
wrapper around the C++ API. There's no sense in pretending this is 
API/ABI stable, it's a thin wrapper around the constantly-in-flux C++ 
one. If people want to write a proper high-level abstraction around it 
(like libLTO) then go right ahead, but that isn't what we've got. For 
example, we removed a whole bunch of functions from the allegedly 
ABI-stable C bindings in the last release, and there's no reason to 
think we won't do it again if we change the C++ API again.

My new policy proposal is that libLTO and libClang remain ABI-stable, 
but the rest of the C bindings are just thin wrappers to allow bindings 
to different languages with no API or ABI guarantee. This matches the 
reality of how they're used today.

Out of tree programs depending on llvm-as-a-library will continue to do 
exactly what they do today; pin themselves to a particular released version.

Any objections?

Nick

>
>     Kostya, as a temporary measure, and until we figure out the right
>     long-term solution, in r148937 I've removed this attribute from the
>     C API's enum, but only that enum. AFAICT, this should have no impact
>     on your usage, and just cause trouble for folks mixing ASan and the
>     C API, which seems very unlikely.
>
>
> Thanks, Chandler!
>
> --kcc
>
>
>     On Mon, Jan 23, 2012 at 12:01 PM, Eli Friedman
>     <eli.friedman at gmail.com <mailto:eli.friedman at gmail.com>> wrote:
>
>         On Mon, Jan 23, 2012 at 11:02 AM, Kostya Serebryany
>         <kcc at google.com <mailto:kcc at google.com>> wrote:
>          > My previous change in include/llvm-c/Core.h that introduced
>         64-bit
>          > Attributes (r148553) caused a warning
>          > while building with MSVC.
>         http://llvm.org/bugs/show_bug.cgi?id=11828
>          > The following patch fixes the problem (use "static const
>         uint64_t" instead
>          > of enum).
>          > Ok to commit?
>          >
>          > --kcc
>          >
>          > Index: include/llvm-c/Core.h
>          >
>         ===================================================================
>          > --- include/llvm-c/Core.h  (revision 148708)
>          > +++ include/llvm-c/Core.h  (working copy)
>          > @@ -92,7 +92,7 @@
>          >  /** Used to get the users and usees of a Value. See the
>         llvm::Use class. */
>          >  typedef struct LLVMOpaqueUse *LLVMUseRef;
>          >
>          > -typedef enum {
>          > +static const uint64_t
>          >      LLVMZExtAttribute       = 1<<0,
>          >      LLVMSExtAttribute       = 1<<1,
>          >      LLVMNoReturnAttribute   = 1<<2,
>          > @@ -119,8 +119,8 @@
>          >      LLVMReturnsTwice = 1 << 29,
>          >      LLVMUWTable = 1 << 30,
>          >      LLVMNonLazyBind = 1U << 31,
>          > -    LLVMAddressSafety = 1ULL << 32
>          > -} LLVMAttribute;
>          > +    LLVMAddressSafety = 1ULL << 32;
>          > +typedef uint64_t LLVMAttribute;
>          >
>          >  typedef enum {
>          >    /* Terminator Instructions */
>
>         Hmm... actually, I'm not sure this is okay; it's a
>         non-binary-compatible change to the C API.
>
>
>     Nick seemed to think that such changes were inevitable and not a big
>     deal. I'm not at all sure. I took the conservative move as I
>     mentioned above and restored the C interface to its previous state
>     as we don't need the functionality there anyways. We can figure out
>     the right way to introduce it now w/o warnings or other noise.
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits