[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