MSVC warning noise on "LLVM_ATTRIBUTE_ALWAYS_INLINE inline void foo()"

__forceinline means that MSVC will always inline that function, that is why
the extra "inline" results in a warning.

I propose:
in llvm/Support/Compiler.h

  #if __has_attribute(always_inline) || LLVM_GNUC_PREREQ(4, 0, 0)
#define LLVM_ATTRIBUTE_ALWAYS_INLINE __attribute__((always_inline))
#elif defined(_MSC_VER) #define LLVM_ATTRIBUTE_ALWAYS_INLINE
__forceinline #else- #define LLVM_ATTRIBUTE_ALWAYS_INLINE+ #define
LLVM_ATTRIBUTE_ALWAYS_INLINE inline #endif

and elsewhere

  LLVM_ATTRIBUTE_ALWAYS_INLINE- inline bool operator==(StringRef LHS,
StringRef RHS) {
+ bool operator==(StringRef LHS, StringRef RHS) {
    return LHS.equals(RHS);
  }

As far as I can tell from online documentation, that will do the correct
thing on MSVC and GCC. For non-MSVC, non-GCC, this will add "inline" in
front of some functions that do not have it right now, notably member
functions that are defined in the class definition (see e.g. StringRef.h
empty()). I will have to test if /that/ doesn't create a warning :wink:

-Johan

>
> Perhaps LLVM_ATTRIBUTE_ALWAYS_INLINE could be defined to "inline" if the
> compiler has no support for always_inline (currently it is set to
> nothing in
> that case) ?
> I think this would allow removal of the "inline" after
> LLVM_ATTRIBUTE_ALWAYS_INLINE.

Wouldn't this cause functions with MSVC that are marked
LLVM_ATTRIBUTE_ALWAYS_INLINE but not 'inline' to not be inlined?

__forceinline means that MSVC will always inline that function, that is why
the extra "inline" results in a warning.

__forceinline is still just a hint to the compiler to do the inline,
but it is a stronger hint that inline
(https://msdn.microsoft.com/en-us/library/bw1hbe6y.aspx). inline is a
keyword in C++ that has additional semantics that __forceinline is not
required to have, such as the fact that an inline function with
external linkage is required to have the same address in all TUs. I am
not comfortable assuming that inline === __forceinline for all
versions of MSVC (including future ones).

~Aaron

OK, thanks for the extra explanation.
I guess LDC will have to go the same route of silencing C4141 warnings (http://llvm.org/viewvc/llvm-project?view=revision&revision=247275).

(sorry for the maillist noise :wink:

  • Johan

OK, thanks for the extra explanation.
I guess LDC will have to go the same route of silencing C4141 warnings
(http://llvm.org/viewvc/llvm-project?view=revision&revision=247275).

That's one option. Another option might be to explore adding an
LLVM_ATTRIBUTE_INLINE_ALWAYS_INLINE macro that does the correct
combination based on compiler usage. I'm not certain whether that will
be an improvement or not (it depends on how often the pattern arises
in the code base, I would guess). Silencing the warning is a valid
option, but the unfortunate part is that the warning is for all
duplicate specifiers, not just inline.

~Aaron