llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.66k stars 11.85k forks source link

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

Closed ldionne closed 5 months ago

ldionne commented 2 years ago

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))];
llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-frontend

shafik commented 2 years ago

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 commented 7 months ago

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

N.B. MSVC changed that in v19.32

frederick-vs-ja commented 6 months ago

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;