<div dir="ltr">OK, thanks for the extra explanation.<div>I guess LDC will have to go the same route of silencing C4141 warnings (<a href="http://llvm.org/viewvc/llvm-project?view=revision&revision=247275">http://llvm.org/viewvc/llvm-project?view=revision&revision=247275</a>).</div><div><br></div><div>(sorry for the maillist noise ;)</div><div><br></div><div>- Johan</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 21, 2015 at 3:57 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Dec 21, 2015 at 6:40 AM, Johan Engelen <<a href="mailto:jbc.engelen@gmail.com">jbc.engelen@gmail.com</a>> wrote:<br>
> On Mon, Dec 21, 2015 at 12:08 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> On Sun, Dec 20, 2015 at 5:57 PM, Johan Engelen <<a href="mailto:jbc.engelen@gmail.com">jbc.engelen@gmail.com</a>><br>
>> wrote:<br>
>> ><br>
>> > Perhaps LLVM_ATTRIBUTE_ALWAYS_INLINE could be defined to "inline" if the<br>
>> > compiler has no support for always_inline (currently it is set to<br>
>> > nothing in<br>
>> > that case) ?<br>
>> > I think this would allow removal of the "inline" after<br>
>> > LLVM_ATTRIBUTE_ALWAYS_INLINE.<br>
>><br>
>> Wouldn't this cause functions with MSVC that are marked<br>
>> LLVM_ATTRIBUTE_ALWAYS_INLINE but not 'inline' to not be inlined?<br>
><br>
><br>
> __forceinline means that MSVC will always inline that function, that is why<br>
> the extra "inline" results in a warning.<br>
<br>
</span>__forceinline is still just a hint to the compiler to do the inline,<br>
but it is a stronger hint that inline<br>
(<a href="https://msdn.microsoft.com/en-us/library/bw1hbe6y.aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-us/library/bw1hbe6y.aspx</a>). inline is a<br>
keyword in C++ that has additional semantics that __forceinline is not<br>
required to have, such as the fact that an inline function with<br>
external linkage is required to have the same address in all TUs. I am<br>
not comfortable assuming that inline === __forceinline for all<br>
versions of MSVC (including future ones).<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
><br>
> I propose:<br>
> in llvm/Support/Compiler.h<br>
><br>
>   #if __has_attribute(always_inline) || LLVM_GNUC_PREREQ(4, 0, 0)<br>
>   #define LLVM_ATTRIBUTE_ALWAYS_INLINE __attribute__((always_inline))<br>
>   #elif defined(_MSC_VER)<br>
>   #define LLVM_ATTRIBUTE_ALWAYS_INLINE __forceinline<br>
>   #else<br>
> - #define LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
> + #define LLVM_ATTRIBUTE_ALWAYS_INLINE inline<br>
>   #endif<br>
><br>
> and elsewhere<br>
><br>
>   LLVM_ATTRIBUTE_ALWAYS_INLINE<br>
> - inline bool operator==(StringRef LHS, StringRef RHS) {<br>
> + bool operator==(StringRef LHS, StringRef RHS) {<br>
>     return LHS.equals(RHS);<br>
>   }<br>
><br>
> As far as I can tell from online documentation, that will do the correct<br>
> thing on MSVC and GCC. For non-MSVC, non-GCC, this will add "inline" in<br>
> front of some functions that do not have it right now, notably member<br>
> functions that are defined in the class definition (see e.g. StringRef.h<br>
> empty()). I will have to test if /that/ doesn't create a warning ;)<br>
><br>
>  -Johan<br>
><br>
><br>
</div></div></blockquote></div><br></div>