[llvm] r180881 - This patch breaks up Wrap.h so that it does not have to include all of

[Filip Pizlo]

Chris already reviewed it, just as I sent that reply!  So both that, and the MCJIT, patch, have now landed. 

-Filip

On May 1, 2013, at 4:11 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:

> Wow, that’s unfortunate.
>  
> I’m not the right person to comment on C API compatibility, so I’ll leave it to someone else to approve your plan.  It sounds reasonable to me.  It looks like most of the other reference definitions follow that idiom already.
>  
> -Andy
>  
> From: Filip Pizlo [mailto:fpizlo at apple.com] 
> Sent: Wednesday, May 01, 2013 3:38 PM
> To: Kaylor, Andrew
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r180881 - This patch breaks up Wrap.h so that it does not have to include all of
>  
> Thanks!
>  
> But it's currently blocked on this: 
>  
> Currently, the LLVM C++ code has a class called llvm::LLVMTargetMachine, while the C API has a struct called LLVMTargetMachine.
> 
> If you're unlucky enough to include both headers, you will get crazy build errors.  Luckily trunk doesn't do this, but I just accidentally caused it to happen because of an unrelated and seemingly benign change.
> 
> We should be able to include both C++ and C headers *somewhere* in the llvm .cpp files, if not broadly in any of them, since otherwise you'd have a really bad time if you wanted to write bindings.
> 
> The offending line in the C API looks like this:
> 
> typedef struct LLVMTargetMachine *LLVMTargetMachineRef;
> 
> I'm assuming that from a source compatibility standpoint, we only care about LLVMTargetMachineRef still working; the fact that the struct was called 'struct LLVMTargetMachine' should not be something that C API clients rely on.  So, the easiest fix to get around this appears to be do just change this to 'struct LLVMOpaqueTargetMachine'.
> 
> -Filip
> 
>  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130501/289e37ee/attachment.html>