Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang reports that __is_array(T[0]) is true #54705

Closed
ldionne opened this issue Apr 1, 2022 · 4 comments · Fixed by #86652
Closed

Clang reports that __is_array(T[0]) is true #54705

ldionne opened this issue Apr 1, 2022 · 4 comments · Fixed by #86652
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@ldionne
Copy link
Member

ldionne commented Apr 1, 2022

Clang currently reports that __is_array(T[0]) is true, which leads to std::is_array<T[0]> being true as well.

Since it's ill-formed, I guess Clang is technically conforming, however it still leads to code like this surprisingly compiling with Clang:

static_assert(!std::is_bounded_array_v<T[0]>);
static_assert(!std::is_unbounded_array_v<T[0]>);
static_assert(std::is_array_v<T[0]>);

So it looks like T[0] is neither a bounded array nor an unbounded array, but it is an array, which is quite confusing. I would suggest Clang either:

  1. Errors out when we try to form T[0] since it's ill-formed, or
  2. At least not report that T[0] is an array from its __is_array builtin (patch provided for this)

Also note that Clang will not match a partial specialization like template <class T, size_t N> struct foo<T[N]> { ... }; when instantiating it with T[0], which seems to support the fact that we really don't want to treat T[0] as an array type.

Here's a potential patch that changes __is_array(T[0]) to false, but I suspect we may want a deeper fix in the compiler.

diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f360dc6e1a23..c7178e097213 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4870,6 +4870,9 @@ static bool EvaluateUnaryTypeTrait(Sema &Self, TypeTrait UTT,
   case UTT_IsFloatingPoint:
     return T->isFloatingType();
   case UTT_IsArray:
+    // We don't want to report T[0] as being an array type.
+    if (ConstantArrayType const* CAT = C.getAsConstantArrayType(T))
+      return CAT->getSize() != 0;
     return T->isArrayType();
   case UTT_IsPointer:
     return T->isAnyPointerType();
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index d576e4388d6f..ce45d1857179 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -23,6 +23,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'}}
@@ -673,7 +674,8 @@ void is_array()
 {
   int t01[T(__is_array(IntAr))];
   int t02[T(__is_array(IntArNB))];
-  int t03[T(__is_array(UnionAr))];
+  int t03[F(__is_array(IntArZero))];
+  int t04[T(__is_array(UnionAr))];
 
   int t10[F(__is_array(void))];
   int t11[F(__is_array(cvoid))];
@ldionne ldionne added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Apr 1, 2022
@llvmbot
Copy link
Member

llvmbot commented Apr 1, 2022

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Apr 1, 2022

Note, if you use -pedantic both gcc and clang will note that zero-sized arrays as an extension e.g.:

<source>:3:42: warning: zero size arrays are an extension [-Wzero-length-array]
static_assert(!std::is_bounded_array<int[0]>::value);
                                         ^
<source>:4:44: warning: zero size arrays are an extension [-Wzero-length-array]
static_assert(!std::is_unbounded_array<int[0]>::value);
                                           ^
<source>:5:34: warning: zero size arrays are an extension [-Wzero-length-array]
static_assert(!std::is_array<int[0]>::value); // implemented as __is_array on Clang/libc++

I have always been curious why we allowed this outside of flexible array members but last time I asked @zygoloid was not sure why.

@jwakely
Copy link
Contributor

jwakely commented Mar 26, 2024

GCC says it's false, MSVC says it's true (see Godbolt).

N.B. MSVC changed that in v19.32

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Apr 17, 2024

Perhaps it makes more sence to treat T[0] as a bounded array. It seems to me that we can "fix" std::is_array.

Proof-of-concept implementation (Godbolt link):

#include <cstddef>
#include <type_traits>

template<class T, bool = std::is_const<const T>::value && !std::is_void<T>::value>
struct ext_is_array_impl { // functions, references, and cv void
    static constexpr bool is_array = false;
    static constexpr bool is_bounded_array = false;
    static constexpr bool is_unbounded_array = false;
};

template<class T, std::size_t N>
struct ext_is_array_impl<T[N], true> {
    static constexpr bool is_array = true;
    static constexpr bool is_bounded_array = true;
    static constexpr bool is_unbounded_array = false;
};

template<class T>
struct ext_is_array_impl<T[], true> {
    static constexpr bool is_array = true;
    static constexpr bool is_bounded_array = false;
    static constexpr bool is_unbounded_array = true;
};

template<class>
struct ext_is_array_impl_extraction_helper {};
template<class T>
struct ext_is_array_impl_extraction_helper<void(*)(T)> { using type = T; };

template<class T>
struct ext_is_array_impl<T, true> { // for non-array objects and extension T[0]
    static constexpr bool is_array = !std::is_same<
        typename std::remove_cv<T>::type,
        typename ext_is_array_impl_extraction_helper<void(*)(T)>::type>::value;
    static constexpr bool is_bounded_array = is_array;
    static constexpr bool is_unbounded_array = false;
};

template<class T>
constexpr bool ext_is_array_v = ext_is_array_impl<T>::is_array;
template<class T>
constexpr bool ext_is_bounded_array_v = ext_is_array_impl<T>::is_bounded_array;
template<class T>
constexpr bool ext_is_unbounded_array_v = ext_is_array_impl<T>::is_unbounded_array;

philnik777 added a commit that referenced this issue May 20, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Fixes #54705
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants