[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/philnik777 closed https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/philnik777 updated
https://github.com/llvm/llvm-project/pull/86652
>From 90f88bdac3dc40635c58f3f67e29354e6a32896e Mon Sep 17 00:00:00 2001
From: Nikolas Klauser
Date: Tue, 26 Mar 2024 12:23:24 +0100
Subject: [PATCH] [Clang] Fix __is_array returning true for zero-sized arrays
---
clang/docs/ReleaseNotes.rst| 2 ++
clang/lib/Sema/SemaExprCXX.cpp | 4
clang/test/SemaCXX/type-traits.cpp | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca06..e668202853710 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -345,6 +345,8 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- ``__is_array`` no longer returns ``true`` for zero-sized arrays. Fixes
(#GH54705).
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81a..93d2b4b259fbc 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait
UTT,
case UTT_IsFloatingPoint:
return T->isFloatingType();
case UTT_IsArray:
+// zero-sized arrays aren't considered arrays in partial specializations,
+// so __is_array shouldn't consider them arrays either.
+if (const auto* CAT = C.getAsConstantArrayType(T))
+ return CAT->getSize() != 0;
return T->isArrayType();
case UTT_IsBoundedArray:
if (!T->isVariableArrayType()) {
diff --git a/clang/test/SemaCXX/type-traits.cpp
b/clang/test/SemaCXX/type-traits.cpp
index 14ec17989ec7c..49df4b668513c 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -25,6 +25,7 @@ typedef Empty EmptyArMB[1][2];
typedef int Int;
typedef Int IntAr[10];
typedef Int IntArNB[];
+typedef Int IntArZero[0];
class Statics { static int priv; static NonPOD np; };
union EmptyUnion {};
union IncompleteUnion; // expected-note {{forward declaration of
'IncompleteUnion'}}
@@ -685,6 +686,7 @@ void is_array()
{
static_assert(__is_array(IntAr));
static_assert(__is_array(IntArNB));
+ static_assert(!__is_array(IntArZero));
static_assert(__is_array(UnionAr));
static_assert(!__is_array(void));
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
// FIXME: the following should be rejected (array of unknown bound and void
are the only allowed incomplete types)
- static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
+ static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
static_assert(!__is_layout_compatible(CStruct, CStructIncomplete));
static_assert(__is_layout_compatible(CStructIncomplete[2],
CStructIncomplete[2]));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/philnik777 updated
https://github.com/llvm/llvm-project/pull/86652
>From 90f88bdac3dc40635c58f3f67e29354e6a32896e Mon Sep 17 00:00:00 2001
From: Nikolas Klauser
Date: Tue, 26 Mar 2024 12:23:24 +0100
Subject: [PATCH] [Clang] Fix __is_array returning true for zero-sized arrays
---
clang/docs/ReleaseNotes.rst| 2 ++
clang/lib/Sema/SemaExprCXX.cpp | 4
clang/test/SemaCXX/type-traits.cpp | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca06..e668202853710 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -345,6 +345,8 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- ``__is_array`` no longer returns ``true`` for zero-sized arrays. Fixes
(#GH54705).
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81a..93d2b4b259fbc 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait
UTT,
case UTT_IsFloatingPoint:
return T->isFloatingType();
case UTT_IsArray:
+// zero-sized arrays aren't considered arrays in partial specializations,
+// so __is_array shouldn't consider them arrays either.
+if (const auto* CAT = C.getAsConstantArrayType(T))
+ return CAT->getSize() != 0;
return T->isArrayType();
case UTT_IsBoundedArray:
if (!T->isVariableArrayType()) {
diff --git a/clang/test/SemaCXX/type-traits.cpp
b/clang/test/SemaCXX/type-traits.cpp
index 14ec17989ec7c..49df4b668513c 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -25,6 +25,7 @@ typedef Empty EmptyArMB[1][2];
typedef int Int;
typedef Int IntAr[10];
typedef Int IntArNB[];
+typedef Int IntArZero[0];
class Statics { static int priv; static NonPOD np; };
union EmptyUnion {};
union IncompleteUnion; // expected-note {{forward declaration of
'IncompleteUnion'}}
@@ -685,6 +686,7 @@ void is_array()
{
static_assert(__is_array(IntAr));
static_assert(__is_array(IntArNB));
+ static_assert(!__is_array(IntArZero));
static_assert(__is_array(UnionAr));
static_assert(!__is_array(void));
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
// FIXME: the following should be rejected (array of unknown bound and void
are the only allowed incomplete types)
- static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
+ static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
static_assert(!__is_layout_compatible(CStruct, CStructIncomplete));
static_assert(__is_layout_compatible(CStructIncomplete[2],
CStructIncomplete[2]));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/philnik777 updated
https://github.com/llvm/llvm-project/pull/86652
>From 90f88bdac3dc40635c58f3f67e29354e6a32896e Mon Sep 17 00:00:00 2001
From: Nikolas Klauser
Date: Tue, 26 Mar 2024 12:23:24 +0100
Subject: [PATCH] [Clang] Fix __is_array returning true for zero-sized arrays
---
clang/docs/ReleaseNotes.rst| 2 ++
clang/lib/Sema/SemaExprCXX.cpp | 4
clang/test/SemaCXX/type-traits.cpp | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca06..e668202853710 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -345,6 +345,8 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- ``__is_array`` no longer returns ``true`` for zero-sized arrays. Fixes
(#GH54705).
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81a..93d2b4b259fbc 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait
UTT,
case UTT_IsFloatingPoint:
return T->isFloatingType();
case UTT_IsArray:
+// zero-sized arrays aren't considered arrays in partial specializations,
+// so __is_array shouldn't consider them arrays either.
+if (const auto* CAT = C.getAsConstantArrayType(T))
+ return CAT->getSize() != 0;
return T->isArrayType();
case UTT_IsBoundedArray:
if (!T->isVariableArrayType()) {
diff --git a/clang/test/SemaCXX/type-traits.cpp
b/clang/test/SemaCXX/type-traits.cpp
index 14ec17989ec7c..49df4b668513c 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -25,6 +25,7 @@ typedef Empty EmptyArMB[1][2];
typedef int Int;
typedef Int IntAr[10];
typedef Int IntArNB[];
+typedef Int IntArZero[0];
class Statics { static int priv; static NonPOD np; };
union EmptyUnion {};
union IncompleteUnion; // expected-note {{forward declaration of
'IncompleteUnion'}}
@@ -685,6 +686,7 @@ void is_array()
{
static_assert(__is_array(IntAr));
static_assert(__is_array(IntArNB));
+ static_assert(!__is_array(IntArZero));
static_assert(__is_array(UnionAr));
static_assert(!__is_array(void));
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
// FIXME: the following should be rejected (array of unknown bound and void
are the only allowed incomplete types)
- static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
+ static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
static_assert(!__is_layout_compatible(CStruct, CStructIncomplete));
static_assert(__is_layout_compatible(CStructIncomplete[2],
CStructIncomplete[2]));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT, case UTT_IsFloatingPoint: return T->isFloatingType(); case UTT_IsArray: +// zero-sized arrays aren't considered arrays in partial specializations, AaronBallman wrote: ```suggestion // Zero-sized arrays aren't considered arrays in partial specializations, ``` https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT, case UTT_IsFloatingPoint: return T->isFloatingType(); case UTT_IsArray: +// zero-sized arrays aren't considered arrays in partial specializations, +// so __is_array shouldn't consider them arrays either. +if (const auto* CAT = C.getAsConstantArrayType(T)) AaronBallman wrote: ```suggestion if (const auto *CAT = C.getAsConstantArrayType(T)) ``` https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/AaronBallman approved this pull request. Realistically, I don't think we can deprecate the extension in general: https://sourcegraph.com/search?q=context:global+%5BA-Za-z0-9_%5D%2B%5Cs%5BA-Za-z0-9_%5D%2B%5C%5B0%5C%5D%3B+-file:.*test.*+-file:.*clang.*+count:10&patternType=regexp&sm=0 I don't think we can even deprecate it for just C++: https://sourcegraph.com/search?q=context:global+%5BA-Za-z0-9_%5D%2B%5Cs%5BA-Za-z0-9_%5D%2B%5C%5B0%5C%5D%3B+-file:.*test.*+-file:.*clang.*+count:10+lang:C%2B%2B+&patternType=regexp&sm=0 but we might be able to deprecate it for uses where the zero-sized array is not the last member of a structure declaration: https://sourcegraph.com/search?q=context:global+%5BA-Za-z0-9_%5D%2B%5Cs%5BA-Za-z0-9_%5D%2B%5C%5B0%5C%5D%3B%5Cs*%5BA-Za-z_%5D+-file:.*test.*+-file:.*clang.*+lang:C+count:10&patternType=regexp&sm=0 (has a lot of false positives like `sizeof foo[0];` and `return foo[0];`) I think we should consider deprecation orthogonal to this patch though. There does not seem to be evidence that this is being used for anything other than implementing traits: https://sourcegraph.com/search?q=context:global+__is_array%5Cb+lang:C%2B%2B+-file:.*test.*+-file:.*clang.*+-file:.*libcxx.*+-file:.*libc%5C%2B%5C%2B.*+-file:.*trait.*+-repo:.*clang.*&patternType=regexp&sm=0 so I think the patch is "correct" insofar as this terrible extension is concerned. Some small nits with the changes, but otherwise LGTM. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
cor3ntin wrote: My 0 cents: - I think deprecating the extension make sense - In the meantime I think the builtin exists to support the library so the patch make sense as is (otherwise we have to prove it is used for something else). My understanding is that "0-length arrays" are mostly an unfortunate attempt at a flexible-member-arrays with sharper edges, so their support in the context of c++ type trait doesn't really make sense. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
philnik777 wrote: @AaronBallman Any thoughts on how we should proceed here? https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
philnik777 wrote:
I'd personally be fine with deprecating it. As I said above, we use it in
libc++ for padding inside `string` like
```c++
struct short_string {
;
char padding[sizeof(value_type) - 1];
;
};
```
That could be refactored relatively easily as
```c++
template
struct padding_t {
char data[Size];
};
template <>
struct padding_t<0> {};
struct short_string {
;
[[no_unique_address]] padding_t padding;
;
};
```
so I don't think that's a use-case worth keeping the extension for.
https://github.com/llvm/llvm-project/pull/86652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
AaronBallman wrote: > Last time I checked, neither GCC nor Clang allowed T[0] to match a partial > specialization for `is_array` so the extension isn't very well extended > :) Oh, I agree, I am learning to loathe this extension the more I look into this. Did you see this lovely bit: > 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 But that's why I'm asking questions about what we *should* be doing with the feature overall. As it currently stands, this extension seems like it is broken in C++ in multiple ways. Fixing a tiny corner of it is fine, but I'd like to know what the overall big picture is so we can know whether the tiny corner changes fit or not. We could fix this by changing partial specializations to care about zero-sized arrays, too. (Personally, I would love to "fix" this by deprecating the extension as the only justification for it I've been able to dig up is as a hack for flexible arrays -- deprecating this extension outside of GNU89 mode has some appeal, barring other information about its value.) https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
jwakely wrote: Last time I checked, neither GCC nor Clang allowed T[0] to match a partial specialization for `is_array` so the extension isn't very well extended :) https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
AaronBallman wrote: > As I've just commented at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114479 > the `__is_array` intrinsic exists to optimize `std::is_array` and so should > match its existing behaviour. If you want to change the behaviour of > `std::is_array` then please do so via an LWG issue, e.g. requesting > clarification of the behaviour for `T[0]`. > > An optimization to speed up compilation of `std::is_array` should not dictate > its behaviour. Errr, I think there's some circular logic here perhaps: > [dcl.array] says that for T[N] the value "N specifies the array bound, i.e., > the number of elements in the array; N shall be greater than zero." > > So T[0] is not a valid array type. Except `T[0]` is a valid array type because it's supported as a language extension in Clang and GCC; that's the whole reason you can spell `T[0]` at all. This boils down to a fundamental question: what makes something an array type in the first place? Is it the declaration syntax or is it the semantics? If it's declaration syntax, then `T[0]` is clearly an array type. If it's semantics, then `T[0]` is not an array type but `std::vector` is. Personally, I think the declaration syntax is what makes something an array type per https://eel.is/c++draft/dcl.array#1 saying "a declaration of the form". Yes, it says the size shall not be zero, but that's the bit we've extended, so it's circular to say "the standard says zero isn't allowed and therefore the library behavior is to return false from std::is_array". So I think I'm arguing for an LWG issue. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
jwakely wrote: As I've just commented at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114479 the `__is_array` intrinsic exists to optimize `std::is_array` and so should match its existing behaviour. If you want to change the behaviour of `std::is_array` then please do so via an LWG issue, e.g. requesting clarification of the behaviour for `T[0]`. An optimization to speed up compilation of `std::is_array` should not dictate its behaviour. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
jwakely wrote: > 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. N.B. MSVC used to have the same bug as well, and it affected their `std::is_array` because MSVC auto-replaces `std::is_array::value` with the intrinsic (and never instantiates the class template). I noted this behaviour in Feb 2022 and they fixed the intrinsic. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
AaronBallman wrote: > Another option would be to to not acknowledge zero-sized arrays in the type > traits at all, just like vector types: https://godbolt.org/z/aP685vz8q. That > may be the most consistent stance on this. Basically "It's non-standard and > doesn't fit neatly in any of the standard categories, so it's in no category > at all." That's a possibility, though I wonder if it's actually user-friendly to issue a diagnostic if trying to use a non-standard type that can't fit the C++ type traits model. It's annoying for folks writing generic code, but so long as the diagnostic is SFINAE-able, maybe it's okay? My concern is that either returning `true` or `false` from any of those traits may give really surprising results to users, whereas a diagnostic makes it clear "you've wandered off the beaten path here and you need to think more carefully about what this all means". It's not quite as far as the idea of making them invalid types and perhaps is a bad idea, but I'm also somewhat okay with the idea of making it harder to use these non-standard extensions in highly generic code. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
kees wrote:
I guess I don't have a strong opinion here, since these helpers are specific to
C++, and I've been generally trying to remove fixed-size 0-sized arrays in C
projects (i.e. the Linux kernel). I do care about C flexible arrays (and their
associated extensions), though. I suspect there is some existing weirdness for
C++ when using static initializers, though. See https://godbolt.org/z/PnYMcxhfM
```
struct S { int a; int b[]; };
static struct S s = { .a = 42, .b = { 10, 20, 30, 40 }, };
static struct S z = { .a = 13, };
```
Specifically `z.b` is a 0-sized flexible array, but `s.b` isn't. Should they
have different results?
https://github.com/llvm/llvm-project/pull/86652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
philnik777 wrote: Another option would be to to not acknowledge zero-sized arrays in the type traits at all, just like vector types: https://godbolt.org/z/aP685vz8q. That may be the most consistent stance on this. Basically "It's non-standard and doesn't fit neatly in any of the standard categories, so it's in no category at all." https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
philnik777 wrote: > 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? I think it may be worth exploring, but I'm not sure it's feasible. We've been using them accidentally a lot in the test suite in libc++ simply because it's really easy to miss that it's an extension, and we're also using it in our `string` implementation for padding. > > 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. > > Yes. An array is an array, regardless of its size. The size is just a storage > characteristic. It'd almost be like arguing that `NaN` isn't a float. I don't think the float analogy really works here. The fact is that zero-sized arrays aren't acknowledged as being valid by the standard, so they don't behave like other arrays. It's _not_ just a storage characteristic. Having `-ffast-math` and then passing `NAN` somewhere seems like a better analogy to me. Things are mostly treated as if `NAN` didn't exist, and `NAN` also doesn't work like other floats. > > I wonder if we should be considering making zero-length arrays into a > > non-conforming extension behind a flag? > > It was never as simple as zero-length arrays. It was also _trailing arrays of > any size_ in structures. Those extensions create huge problems with being > able to reason about the characteristics of arrays, and we do have a flag for > this: `-fstrict-flex-arrays=3`. It is designed to disable all of the "treat > it like a flexible array" logic for the old "fake flexible array" objects. > But a flexible array is _still an array_. And also note that its size can > _change at runtime_ (see the `counted_by` attribute), which even more clearly > argues that you can't claim a flexible array isn't an array. I'm not sure how the array being able to change its size at runtime makes it harder to claim it isn't one? non-zero-sized arrays don't do that. It's actually another thing that is different from the rest of the arrays. I'm also not sure that it makes a ton of sense to talk about flexible arrays, since they aren't part of the type system, but `T[0]` happen to become flexible arrays if it is at the end of a struct (IIUC). Another option (that I haven't thought deeply about): Make `T[0]` not be a type at all, i.e. make it illegal to `decltype`, `sizeof` etc. uses of it. I don't know how breaking that would be, and it's probably just as confusing as the current state of things, or even more so. Just wanted to throw it out. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
kees wrote: > 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. Yes. An array is an array, regardless of its size. The size is just a storage characteristic. It'd almost be like arguing that `NaN` isn't a float. > I wonder if we should be considering making zero-length arrays into a > non-conforming extension behind a flag? It was never as simple as zero-length arrays. It was also _trailing arrays of any size_ in structures. Those extensions create huge problems with being able to reason about the characteristics of arrays, and we do have a flag for this: `-fstrict-flex-arrays=3`. It is designed to disable all of the "treat it like a flexible array" logic for the old "fake flexible array" objects. But a flexible array is _still an array_. And also note that its size can _change at runtime_ (see the `counted_by` attribute), which even more clearly argues that you can't claim a flexible array isn't an array. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
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
> int func() {
> return 1;
> }
>
> template
> 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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
philnik777 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
int func() {
return 1;
}
template
int func() {
return 0;
}
```
If clang accepted `int[0]` in this context the observable behaviour would
change.
> > 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.
https://github.com/llvm/llvm-project/pull/86652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
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. > 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.) > 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 https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
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 inline constexpr bool is_array_v = false; template inline constexpr bool is_array_v = true; template inline constexpr bool is_array_v = 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/AaronBallman requested changes to this pull request. 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.) 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. 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? https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n) static_assert(!__is_layout_compatible(EnumForward, int)); static_assert(!__is_layout_compatible(EnumClassForward, int)); // FIXME: the following should be rejected (array of unknown bound and void are the only allowed incomplete types) - static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete)); + static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete)); AaronBallman wrote: Spurious whitespace change. https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 5b544b511c7133fcb26a5c563b746a4baefb38d6 90f88bdac3dc40635c58f3f67e29354e6a32896e -- clang/lib/Sema/SemaExprCXX.cpp clang/test/SemaCXX/type-traits.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 93d2b4b259..46dd14a871 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -5145,7 +5145,7 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT, case UTT_IsArray: // zero-sized arrays aren't considered arrays in partial specializations, // so __is_array shouldn't consider them arrays either. -if (const auto* CAT = C.getAsConstantArrayType(T)) +if (const auto *CAT = C.getAsConstantArrayType(T)) return CAT->getSize() != 0; return T->isArrayType(); case UTT_IsBoundedArray: `` https://github.com/llvm/llvm-project/pull/86652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Nikolas Klauser (philnik777)
Changes
Fixes #54705
---
Full diff: https://github.com/llvm/llvm-project/pull/86652.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/lib/Sema/SemaExprCXX.cpp (+4)
- (modified) clang/test/SemaCXX/type-traits.cpp (+3-1)
``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca065..e6682028537101 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -345,6 +345,8 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- ``__is_array`` no longer returns ``true`` for zero-sized arrays. Fixes
(#GH54705).
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81ac..93d2b4b259fbc3 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait
UTT,
case UTT_IsFloatingPoint:
return T->isFloatingType();
case UTT_IsArray:
+// zero-sized arrays aren't considered arrays in partial specializations,
+// so __is_array shouldn't consider them arrays either.
+if (const auto* CAT = C.getAsConstantArrayType(T))
+ return CAT->getSize() != 0;
return T->isArrayType();
case UTT_IsBoundedArray:
if (!T->isVariableArrayType()) {
diff --git a/clang/test/SemaCXX/type-traits.cpp
b/clang/test/SemaCXX/type-traits.cpp
index 14ec17989ec7c7..49df4b668513c0 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -25,6 +25,7 @@ typedef Empty EmptyArMB[1][2];
typedef int Int;
typedef Int IntAr[10];
typedef Int IntArNB[];
+typedef Int IntArZero[0];
class Statics { static int priv; static NonPOD np; };
union EmptyUnion {};
union IncompleteUnion; // expected-note {{forward declaration of
'IncompleteUnion'}}
@@ -685,6 +686,7 @@ void is_array()
{
static_assert(__is_array(IntAr));
static_assert(__is_array(IntArNB));
+ static_assert(!__is_array(IntArZero));
static_assert(__is_array(UnionAr));
static_assert(!__is_array(void));
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
// FIXME: the following should be rejected (array of unknown bound and void
are the only allowed incomplete types)
- static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
+ static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
static_assert(!__is_layout_compatible(CStruct, CStructIncomplete));
static_assert(__is_layout_compatible(CStructIncomplete[2],
CStructIncomplete[2]));
}
``
https://github.com/llvm/llvm-project/pull/86652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix __is_array returning true for zero-sized arrays (PR #86652)
https://github.com/philnik777 created
https://github.com/llvm/llvm-project/pull/86652
Fixes #54705
>From 90f88bdac3dc40635c58f3f67e29354e6a32896e Mon Sep 17 00:00:00 2001
From: Nikolas Klauser
Date: Tue, 26 Mar 2024 12:23:24 +0100
Subject: [PATCH] [Clang] Fix __is_array returning true for zero-sized arrays
---
clang/docs/ReleaseNotes.rst| 2 ++
clang/lib/Sema/SemaExprCXX.cpp | 4
clang/test/SemaCXX/type-traits.cpp | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7fbe2fec6ca065..e6682028537101 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -345,6 +345,8 @@ Bug Fixes in This Version
- Fixes an assertion failure on invalid code when trying to define member
functions in lambdas.
+- ``__is_array`` no longer returns ``true`` for zero-sized arrays. Fixes
(#GH54705).
+
Bug Fixes to Compiler Builtins
^^
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c34a40fa7c81ac..93d2b4b259fbc3 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5143,6 +5143,10 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait
UTT,
case UTT_IsFloatingPoint:
return T->isFloatingType();
case UTT_IsArray:
+// zero-sized arrays aren't considered arrays in partial specializations,
+// so __is_array shouldn't consider them arrays either.
+if (const auto* CAT = C.getAsConstantArrayType(T))
+ return CAT->getSize() != 0;
return T->isArrayType();
case UTT_IsBoundedArray:
if (!T->isVariableArrayType()) {
diff --git a/clang/test/SemaCXX/type-traits.cpp
b/clang/test/SemaCXX/type-traits.cpp
index 14ec17989ec7c7..49df4b668513c0 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -25,6 +25,7 @@ typedef Empty EmptyArMB[1][2];
typedef int Int;
typedef Int IntAr[10];
typedef Int IntArNB[];
+typedef Int IntArZero[0];
class Statics { static int priv; static NonPOD np; };
union EmptyUnion {};
union IncompleteUnion; // expected-note {{forward declaration of
'IncompleteUnion'}}
@@ -685,6 +686,7 @@ void is_array()
{
static_assert(__is_array(IntAr));
static_assert(__is_array(IntArNB));
+ static_assert(!__is_array(IntArZero));
static_assert(__is_array(UnionAr));
static_assert(!__is_array(void));
@@ -1804,7 +1806,7 @@ void is_layout_compatible(int n)
static_assert(!__is_layout_compatible(EnumForward, int));
static_assert(!__is_layout_compatible(EnumClassForward, int));
// FIXME: the following should be rejected (array of unknown bound and void
are the only allowed incomplete types)
- static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
+ static_assert(__is_layout_compatible(CStructIncomplete, CStructIncomplete));
static_assert(!__is_layout_compatible(CStruct, CStructIncomplete));
static_assert(__is_layout_compatible(CStructIncomplete[2],
CStructIncomplete[2]));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
