[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)

Nikolas Klauser via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 26 05:28:26 PDT 2024


philnik777 wrote:

> My primary question is: then what is it?
> 
> We return true for `__is_aggregrate` (https://godbolt.org/z/67zjeo7Mj), and an aggregate is an array or class type (https://eel.is/c++draft/dcl.init.aggr#1). This isn't a class type... but it is an aggregate... so it must be an array? (We also don't claim it's a pointer or a reference currently... so this thing will be basically invisible to type traits.)

According to the spec it's ill-formed, so I'm not sure it falls under any sensible category.

> I would think it is an array given that it uses array syntax for declarations and array semantics for accesses, but it's also not an array that's usable so I can see why others say it's not an array. Personally, I think Clang's behavior here makes the most sense -- we claim it's an array, we also claim it's a bounded array, and we claim its extent is zero (https://godbolt.org/z/4GdYTh4GG), and that matches the declaration for the type. So with this change, I'm worried about how type traits can possibly reason about this type -- I'd like to understand better why other implementations decided this isn't an array and what it's classified as with their type traits. Assuming that logic is compelling, there's more work needed here to handle things like claiming it's a bounded array but not an array.

For MSVC it's probably because they don't support `T[0]`. The main reason I don't think it should be considered an array for the type traits is that it doesn't behave like other arrays. e.g. the implementation that's used without the builtin is
```c++
template <class T>
inline constexpr bool is_array_v = false;

template <class T>
inline constexpr bool is_array_v<T[]> = true;

template <class T, size_t Size>
inline constexpr bool is_array_v<T[Size]> = true;
```

For `T[0]` this returns false. 

Another example

> Also, do we need an ABI tag for folks to get the old behavior given that this change almost certainly will impact ABI through type traits?

We don't use the trait currently in libc++ because of this bug and GCC only added it in trunk (and have the same bug), so probably not.


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


More information about the cfe-commits mailing list