[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 11:42:41 PDT 2024


philnik777 wrote:

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

Not really. Zero-sized arrays should probably have been supported by C++ all along, but it's too late to change it now.

> 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.)

I'm not sure what you're referring to. AFAICT `T[]` at the end of a struct denotes a flexible array member, but `T[0]` doesn't. Or does it?

>     * We should probably change the value of `__is_bounded_array` at the same time as `__is_array`.

Yes. I'll update the PR accordingly.

>     * What should the behavior of `__is_aggregate` be?

I guess it should be true? I can't think of any case where a zero-sized array would be different from a non-zero-sized one at least. I don't have a strong opinion either way though.

>     * What about other type traits (`__is_compound`, `__is_abstract`, etc)? Basically, what's the principle we're using for this type?

Maybe we could think of it as array-like? i.e. it has the same traits as an array except that it's not one. I think the semantics only really break down for array-specific features. Just to get a sense, I've gone through a few traits:
- `is_compound`: it seems pretty clear to me that it is a compound type, since `T[0]` is a distinct type from `T`, but contains a type `T` in its definition.
- `is_class`: It seems pretty clear to me that it's not a class. I don't think anybody would disagree here.
- `is_abstract`: You can define a variable of it, so it's not. It's also not a class type.
- `is_object`: I think it is, but this could be argued either way. The best thing I could come up with is that it's closer to an array than it is to a function or reference type.

`is_empty` seems like an interesting case. It's the smallest type there is, with `sizeof(int[0]) == 0`, but it's not considered empty by any implementation. MSVC rejects the `sizeof` call though. I think it shouldn't be considered empty though, since `is_empty` kind-of implies that the type is a class 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.

Sounds good.

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


More information about the cfe-commits mailing list