[clang] [libcxx] [C++17] Support __GCC_[CON|DE]STRUCTIVE_SIZE (PR #89446)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 23 11:30:57 PDT 2024


================
@@ -89,6 +89,19 @@ sections with improvements to Clang's support for those languages.
 C++ Language Changes
 --------------------
 
+C++17 Feature Support
+^^^^^^^^^^^^^^^^^^^^^
+- Clang now exposes ``__GCC_DESTRUCTIVE_SIZE`` and ``__GCC_CONSTRUCTIVE_SIZE``
+  predefined macros to support standard library implementations of
+  ``std::hardware_destructive_interference_size`` and
+  ``std::hardware_constructive_interference_size``, respectively. These macros
+  are predefined in all C and C++ language modes. These macros can be
+  overridden on the command line with ``-D``, if desired. The values the macros
----------------
AaronBallman wrote:

I want to make sure I'm on the same page with what you're asking.
```
struct alignas(__GCC_CONSTRUCTIVE_SIZE) S {}; // #1
struct alignas(std::hardware_constructive_interference_size) T {}; // #2

static_assert(__GCC_CONSTRUCTIVE_SIZE > 0);  // #3
static_assert(std::hardware_constructive_interference_size > 0); // #4
```
There is nothing wrong with either (3) or (4) and I don't think we should diagnose that kind of use. So "any use in a header file" is too strict IMO. The problem with (1) and (2) is that it's a fragile diagnostic unless we're willing to put in significant effort.

The easy way to do (1) is to see if the operand to `alignas` (within a header) comes from a macro and see if the macro was named `__GCC_CONSTRUCTIVE_SIZE` (or destructive). However, that misses these kinds of scenarios:
```
#define DERP __GCC_CONSTRUCTIVE_SIZE
struct alignas(DERP) S {};
struct alignas(__GCC_CONSTRUCTIVE_SIZE + 0) T {};
```
(I'm not worried about people doing arithmetic in `alignas` getting false negatives, they earned them. But I am worried we're going to have to do macro chasing because of `#if` use.)

Resolving names in the STL is non-trivial thanks to inline namespaces, which makes (2) a bit of a challenge. But (2) runs into even worse issues than (1):
```
constexpr size_t MySize = std::hardware_constructive_interference_size;
struct alignas(MySize) S {};
```
and we're no longer doing macro chasing, now we're trying to unpack constant expressions.

Further, if the request is "ABI issues in general", all bets are off because people can do fantastic things like:
```
struct S {
  int array[std::hardware_constructive_interference_size];
};
```
and now we're playing whack-a-mole trying to identify safe vs unsafe uses in header files.

So I think the only thing that's reasonable to consider would be
```
struct alignas(__GCC_CONSTRUCTIVE_SIZE) S {};
struct alignas(std::hardware_constructive_interference_size) T{};
```
and nothing else, but that doesn't actually address your concern about catching ABI issues. FWIW, I think @cor3ntin is correct in that we don't really make special cases for all the other potentially ABI-terrible ideas people can come up with in a header file and there's nothing particularly special about this one.

We already have [ODR checking facilities](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fsanitize-address-use-odr-indicator) and that seems sufficient to catch issues that happen in practice. (Note, I'm not saying we should never add a diagnostic to help users catch potential ODR issues earlier. But I think it's not necessary for this PR to move forward and we should consider such diagnostics in a broader context than just this one narrow case.)

https://github.com/llvm/llvm-project/pull/89446


More information about the cfe-commits mailing list