[llvm] revisions to export annotation macros to avoid compiler warnings

[andrurogerz]

Purpose

Revise the LLVM_ABI family of macros defined in llvm/Support/Compiler.h to eliminate GCC compiler warnings that appear when more of the codebase is annotated.

Overview

This patch changes the annotation behavior in two ways:

1. Switches the macros to explicitly use the __annotation__((visibility("default"))) syntax rather than rely on the pre-existing LLVM_ATTRIBUTE_VISIBILITY_DEFAULT. The LLVM_ATTRIBUTE_VISIBILITY_DEFAULT macro resolves to [[gnu::visibility("default")]] on most compiler versions, which is permitted in fewer locations (such as annotating friend function declarations) and produces warnings.
2. Defines LLVM_TEMPLATE_ABI to nothing when compiling with GCC. This avoids compiler warnings about redefined attributes on explicitly instantiated template types.

Background

Additional context on the effort to annotate LLVM's public interface is in this discourse.

Without this change, compiling LLVM with GCC will produce -Wattributes warnings in a number of cases once more of the codebase has been annotated. Specifically:

warning: attribute ignored [-Wattributes] note: an attribute that appertains to a friend declaration that is not a definition is ignored

warning: type attributes ignored after type is already defined [-Wattributes]

Neither of these warnings are present when compiling with Clang.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Andrew Rogers (andrurogerz)

Changes

Purpose

Revise the LLVM_ABI family of macros defined in llvm/Support/Compiler.h to eliminate GCC compiler warnings that appear when more of the codebase is annotated.

Overview

This patch changes the annotation behavior in two ways:

1. Switches the macros to explicitly use the __annotation__((visibility("default"))) syntax rather than rely on the pre-existing LLVM_ATTRIBUTE_VISIBILITY_DEFAULT. The LLVM_ATTRIBUTE_VISIBILITY_DEFAULT macro resolves to [[gnu::visibility("default")]] on most compiler versions, which is permitted in fewer locations (such as annotating friend function declarations) and produces warnings.
2. Defines LLVM_TEMPLATE_ABI to nothing when compiling with GCC. This avoids compiler warnings about redefined attributes on explicitly instantiated template types.

Background

Additional context on the effort to annotate LLVM's public interface is in this discourse.

Without this change, compiling LLVM with GCC produces -Wattributes warnings in a number of cases when more of the codebase has been annotated. None of these warnings are present when compiling with Clang.

---

Full diff: https://github.com/llvm/llvm-project/pull/135995.diff

1 Files Affected:

• (modified) llvm/include/llvm/Support/Compiler.h (+23-8)

diff --git a/llvm/include/llvm/Support/Compiler.h b/llvm/include/llvm/Support/Compiler.h
index d265d864228ca..baa5fb5c6b97b 100644
--- a/llvm/include/llvm/Support/Compiler.h
+++ b/llvm/include/llvm/Support/Compiler.h
@@ -197,17 +197,32 @@
 #define LLVM_EXPORT_TEMPLATE
 #endif
 #define LLVM_ABI_EXPORT __declspec(dllexport)
-#elif defined(__ELF__) || defined(__MINGW32__) || defined(_AIX) ||             \
-    defined(__MVS__)
-#define LLVM_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
-#define LLVM_TEMPLATE_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#elif (defined(__ELF__) || defined(__MINGW32__) || defined(_AIX) ||            \
+       defined(__MVS__)) &&                                                    \
+    __has_attribute(visibililty)
+// Use __attribute__((visibility(""))) syntax for visibility rather than
+// [[gnu::visibility("")]] because compilers are more permissive with its
+// placement.
+#define LLVM_ABI __attribute__((visibility("default")))
+#if defined(__GNUC__) && !defined(__clang__)
+// GCC produces warnings on visibility attributes applied to some templates.
+#define LLVM_TEMPLATE_ABI
+#else
+#define LLVM_TEMPLATE_ABI __attribute__((visibility("default")))
+#endif
+#define LLVM_EXPORT_TEMPLATE
+#define LLVM_ABI_EXPORT __attribute__((visibility("default")))
+#elif (defined(__MACH__) || defined(__WASM__) || defined(__EMSCRIPTEN__)) &&   \
+    __has_attribute(visibility)
+#define LLVM_ABI __attribute__((visibility("default")))
+#define LLVM_TEMPLATE_ABI
 #define LLVM_EXPORT_TEMPLATE
-#define LLVM_ABI_EXPORT LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
-#elif defined(__MACH__) || defined(__WASM__) || defined(__EMSCRIPTEN__)
-#define LLVM_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define LLVM_ABI_EXPORT __attribute__((visibility("default")))
+#else
+#define LLVM_ABI
 #define LLVM_TEMPLATE_ABI
 #define LLVM_EXPORT_TEMPLATE
-#define LLVM_ABI_EXPORT LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define LLVM_ABI_EXPORT
 #endif
 #else
 #define LLVM_ABI

[andrurogerz]

@compnerd would you mind having a look?

[compnerd]

__has_attribute is a clang extension, and would generally be defined as follows:

#if !defined(__has_attribute)
#define __has_attribute(attribute) 0
#endif

This basically means that the compiler which would previously attempted to use the GNU syntax would no longer do so if it is not clang or does not implement the clang extensions (or does not enable them by default).

[compnerd]

This is not a compilers thing IMO. This is because the C++ generalised attributes have stricter positioning requirements. The GNU attribute parsing is far more complicated due to the long history and its organic growth. That is to say, I think that this is a property of the attribute spelling itself.

[andrurogerz]

Makes sense; will revise this comment.

[andrurogerz]

fwiw, clang is happy with the [[gnu::visibility("")]] attribute in places that gcc is not, so the comment isn't exactly wrong. Point taken, though.

[compnerd]

Ouch! Is this really limited to just GCC? What about QCC? What about ICC (or I suppose the now deprecated ICC)? What about xLC? That is, is this potentially a clang extension?

[andrurogerz]

I suppose I could just invert this and make it clang-only? I don't think it is a clang extension, I just think clang is less whiny about attributes. I've only tested with clang and gcc.

[compnerd]

This block seems like a pretty complicated way to say:

#if !defined(_WIN32)
# if __has_attribute(__visibility)
#   define LLVM_ABI __attribute__((__visibility__("default")))
#   define LLVM_ABI_EXPORT __attribute__((__visibility__("default")))
#   if defined(__clang__)
#     define LLVM_TEMPLATE_ABI __attribute__((__visibility__("default")))
#   else
#     define LLVM_TEMPLATE_ABI
#   endif
# endif
#else

[andrurogerz]

I think I will abandon this change in favor of #136595.

[andrurogerz]

Abandoning this one in favor of some other options.