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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 26 09:43:01 PDT 2024


AaronBallman wrote:

> > > According to the spec it's ill-formed, so I'm not sure it falls under any sensible category.
> > 
> > 
> > It's an extension we support so it's up to us to decide what sensible is.
> 
> Sure. If we end up putting it in the array category we should probably coordinate with GCC.
> 
> > > For T[0] this returns false.
> > 
> > 
> > Understood, but why is this not the bug to be fixed? (It would also fix the `takes_an_array` case as well.)
> 
> Because I don't think we can. AFAICT this is valid C++:
> 
> ```c++
> template <unsigned Size, class = int[Size]>
> int func() {
>   return 1;
> }
> 
> template <unsigned Size>
> int func() {
>   return 0;
> }
> ```
> 
> If clang accepted `int[0]` in this context the observable behaviour would change.

Hmm, I think you're right: https://godbolt.org/z/PjGoc3Yob, the `static_assert(func<0>() == 0);` case would start to fail to compile, similar to the code using `func<1>()`.

This doesn't leave us with good options, does it? :-(

I think we need to consider the following things:

* Should `Ty[0]` and `Ty[]` be handled the same way? (Remember, we support `[0]` in structures as a sort of flexible array member and we support flexible array members in C++ as an extension.)
* We should probably change the value of `__is_bounded_array` at the same time as `__is_array`.
* What should the behavior of `__is_aggregate` be?
* What about other type traits (`__is_compound`, `__is_abstract`, etc)? Basically, what's the principle we're using for this type?

> > > 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.
> > 
> > 
> > For example, it seems to be used in Unreal Engine: https://github.com/windystrife/UnrealEngine_NVIDIAGameWorks/blob/b50e6338a7c5b26374d66306ebc7807541ff815e/Engine/Source/Runtime/Core/Public/Templates/UnrealTemplate.h#L128
> 
> If you think it's a significant enough change that we should add an ABI tag I can do that.

I don't have a good feeling one way or the other yet, so let's hold off for the moment and see what the final scope of the changes are before making a decision.

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


More information about the cfe-commits mailing list