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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 27 06:43:45 PDT 2024


AaronBallman wrote:

> > 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?

It doesn't per spec but it does as a common compiler extension: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

So my question is more that, if `T[0]` isn't considered an array because it has no size, should `T[]` be considered the same way?

> > ```
> > * 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.

I'm not entirely certain of how this particular type trait is used in practice.  Based on https://sourcegraph.com/search?q=context:global+lang:C%2B%2B+std::is_aggregate+-file:.*libcxx.*+-file:.*test.*+-file:.*clang.*+-file:.*gcc.*&patternType=keyword&sm=0 it seems that most uses for this trait are to decide whether you can use aggregate initialization or not, and `Ty[0]` can be "used" in empty aggregate initialization (e.g., `int x[0] = {};`) so perhaps returning `true` makes sense? But then again, an aggregate is a class or array type, so the fact that `Ty[0]` will be reported as `false` for both of those doesn't seem ideal. CC @ldionne @mordante @cor3ntin for more opinions

> > ```
> > * 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.

My natural inclination is that it is array-like, but... that just makes me want `__is_array` to return `true` for it all the more. That's... what it is. I wonder if we should be considering making zero-length arrays into a non-conforming extension behind a flag? e.g., `-fzero-sized-arrays` and then it does report true for `__is_array`, `__is_bounded_array`, and handles template specializations, etc as though it were really an array. That solves two problems: 1) we can make the feature regular as opposed to having so many sharp edges, 2) it pushes a surprising and questionable extension behind an opt-in feature flag, and it creates at least one problem: 1) potentially would change behavior of existing code in C++. So maybe this isn't a tenable idea, but do we think it's worth exploring?

Oh wow, and it gets even more surprising... adding a zero-length array to a structure *reduces the size of the structure* (in C++): https://godbolt.org/z/5E4YsT1Ye

CC @nickdesaulniers @kees @bwendling for more opinions from people who live with this kind of extension...

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


More information about the cfe-commits mailing list