PR44406: Follow behavior of array bound constant folding in more rece…

…nt versions of GCC.

Old GCC used to aggressively fold VLAs to constant-bound arrays at block
scope in GNU mode. That's non-conforming, and more modern versions of
GCC only do this at file scope. Update Clang to do the same.

Also promote the warning for this from off-by-default to on-by-default
in all cases; more recent versions of GCC likewise warn on this by
default.

This is still slightly more permissive than GCC, as pointed out in
PR44406, as we still fold VLAs to constant arrays in structs, but that
seems justifiable given that we don't support VLA-in-struct (and don't
intend to ever support it), but GCC does.

Differential Revision: https://reviews.llvm.org/D89523

clang/docs/UsersManual.rst

@@ -2502,10 +2502,6 @@ Differences between all ``c*`` and ``gnu*`` modes:
 -  The Apple "blocks" extension is recognized by default in ``gnu*`` modes
    on some platforms; it can be enabled in any mode with the ``-fblocks``
    option.
--  Arrays that are VLA's according to the standard, but which can be
-   constant folded by the frontend are treated as fixed size arrays.
-   This occurs for things like "int X[(1, 2)];", which is technically a
-   VLA. ``c*`` modes are strictly compliant and treat these as VLAs.
 
 Differences between ``*89`` and ``*94`` modes:
 
@@ -2594,10 +2590,12 @@ Intentionally unsupported GCC extensions
    the extension appears to be rarely used. Note that clang *does*
    support flexible array members (arrays with a zero or unspecified
    size at the end of a structure).
--  clang does not have an equivalent to gcc's "fold"; this means that
-   clang doesn't accept some constructs gcc might accept in contexts
-   where a constant expression is required, like "x-x" where x is a
-   variable.
+-  GCC accepts many expression forms that are not valid integer constant
+   expressions in bit-field widths, enumerator constants, case labels,
+   and in array bounds at global scope. Clang also accepts additional
+   expression forms in these contexts, but constructs that GCC accepts due to
+   simplifications GCC performs while parsing, such as ``x - x`` (where ``x`` is a
+   variable) will likely never be accepted by Clang.
 -  clang does not support ``__builtin_apply`` and friends; this extension
    is extremely obscure and difficult to implement reliably.
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -137,8 +137,9 @@ def err_vla_decl_has_static_storage : Error<
   "variable length array declaration cannot have 'static' storage duration">;
 def err_vla_decl_has_extern_linkage : Error<
   "variable length array declaration cannot have 'extern' linkage">;
-def ext_vla_folded_to_constant : Extension<
-  "variable length array folded to constant array as an extension">, InGroup<GNUFoldingConstant>;
+def ext_vla_folded_to_constant : ExtWarn<
+  "variable length array folded to constant array as an extension">,
+  InGroup<GNUFoldingConstant>;
 def err_vla_unsupported : Error<
   "variable length arrays are not supported for the current target">;
 def note_vla_unsupported : Note<
@@ -5474,8 +5475,6 @@ def warn_flag_enum_constant_out_of_range : Warning<
   "enumeration value %0 is out of range of flags in enumeration type %1">,
   InGroup<FlagEnum>;
 
-def warn_illegal_constant_array_size : Extension<
-  "size of static array must be an integer constant expression">;
 def err_vm_decl_in_file_scope : Error<
   "variably modified type declaration not allowed at file scope">;
 def err_vm_decl_has_extern_linkage : Error<

clang/lib/Sema/SemaDecl.cpp

@@ -5932,9 +5932,14 @@ static QualType TryToFixInvalidVariablyModifiedType(QualType T,
   const VariableArrayType* VLATy = dyn_cast<VariableArrayType>(T);
   if (!VLATy)
     return QualType();
-  // FIXME: We should probably handle this case
-  if (VLATy->getElementType()->isVariablyModifiedType())
-    return QualType();
+
+  QualType ElemTy = VLATy->getElementType();
+  if (ElemTy->isVariablyModifiedType()) {
+    ElemTy = TryToFixInvalidVariablyModifiedType(ElemTy, Context,
+                                                 SizeIsNegative, Oversized);
+    if (ElemTy.isNull())
+      return QualType();
+  }
 
   Expr::EvalResult Result;
   if (!VLATy->getSizeExpr() ||
@@ -5950,16 +5955,18 @@ static QualType TryToFixInvalidVariablyModifiedType(QualType T,
   }
 
   // Check whether the array is too large to be addressed.
-  unsigned ActiveSizeBits
-    = ConstantArrayType::getNumAddressingBits(Context, VLATy->getElementType(),
-                                              Res);
+  unsigned ActiveSizeBits =
+      (!ElemTy->isDependentType() && !ElemTy->isVariablyModifiedType() &&
+       !ElemTy->isIncompleteType() && !ElemTy->isUndeducedType())
+          ? ConstantArrayType::getNumAddressingBits(Context, ElemTy, Res)
+          : Res.getActiveBits();
   if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) {
     Oversized = Res;
     return QualType();
   }
 
-  return Context.getConstantArrayType(
-      VLATy->getElementType(), Res, VLATy->getSizeExpr(), ArrayType::Normal, 0);
+  return Context.getConstantArrayType(ElemTy, Res, VLATy->getSizeExpr(),
+                                      ArrayType::Normal, 0);
 }
 
 static void
@@ -5985,7 +5992,13 @@ FixInvalidVariablyModifiedTypeLoc(TypeLoc SrcTL, TypeLoc DstTL) {
   ArrayTypeLoc DstATL = DstTL.castAs<ArrayTypeLoc>();
   TypeLoc SrcElemTL = SrcATL.getElementLoc();
   TypeLoc DstElemTL = DstATL.getElementLoc();
-  DstElemTL.initializeFullCopy(SrcElemTL);
+  if (VariableArrayTypeLoc SrcElemATL =
+          SrcElemTL.getAs<VariableArrayTypeLoc>()) {
+    ConstantArrayTypeLoc DstElemATL = DstElemTL.castAs<ConstantArrayTypeLoc>();
+    FixInvalidVariablyModifiedTypeLoc(SrcElemATL, DstElemATL);
+  } else {
+    DstElemTL.initializeFullCopy(SrcElemTL);
+  }
   DstATL.setLBracketLoc(SrcATL.getLBracketLoc());
   DstATL.setSizeExpr(SrcATL.getSizeExpr());
   DstATL.setRBracketLoc(SrcATL.getRBracketLoc());
@@ -6115,7 +6128,7 @@ Sema::CheckTypedefForVariablyModifiedType(Scope *S, TypedefNameDecl *NewTD) {
                                                       SizeIsNegative,
                                                       Oversized);
       if (FixedTInfo) {
-        Diag(NewTD->getLocation(), diag::warn_illegal_constant_array_size);
+        Diag(NewTD->getLocation(), diag::ext_vla_folded_to_constant);
         NewTD->setTypeSourceInfo(FixedTInfo);
       } else {
         if (SizeIsNegative)
@@ -7984,7 +7997,7 @@ void Sema::CheckVariableDeclarationType(VarDecl *NewVD) {
       return;
     }
 
-    Diag(NewVD->getLocation(), diag::warn_illegal_constant_array_size);
+    Diag(NewVD->getLocation(), diag::ext_vla_folded_to_constant);
     NewVD->setType(FixedT);
     NewVD->setTypeSourceInfo(FixedTInfo);
   }
@@ -16675,7 +16688,7 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T,
                                                     SizeIsNegative,
                                                     Oversized);
     if (FixedTInfo) {
-      Diag(Loc, diag::warn_illegal_constant_array_size);
+      Diag(Loc, diag::ext_vla_folded_to_constant);
       TInfo = FixedTInfo;
       T = FixedTInfo->getType();
     } else {

clang/lib/Sema/SemaType.cpp

@@ -2273,13 +2273,9 @@ static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
     }
   } Diagnoser(VLADiag, VLAIsError);
 
-  // FIXME: GCC does *not* allow folding here in general; see PR44406.
-  // For GCC compatibility, we should remove this folding and leave it to
-  // TryFixVariablyModifiedType to convert VLAs to constant array types.
   ExprResult R = S.VerifyIntegerConstantExpression(
       ArraySize, &SizeVal, Diagnoser,
-      (S.LangOpts.GNUMode || S.LangOpts.OpenCL) ? Sema::AllowFold
-                                                : Sema::NoFold);
+      S.LangOpts.OpenCL ? Sema::AllowFold : Sema::NoFold);
   if (Diagnoser.IsVLA)
     return ExprResult();
   return R;

clang/test/CXX/basic/basic.types/p10.cpp

@@ -139,6 +139,7 @@ constexpr int f(ArrBad) { return 0; } // expected-error {{1st parameter type 'Ar
 constexpr int arb(int n) {
   int a[n]; // expected-error {{variable of non-literal type 'int [n]' cannot be defined in a constexpr function}}
 }
+// expected-warning@+1 {{variable length array folded to constant array as an extension}}
 constexpr long Overflow[ // expected-error {{constexpr variable cannot have non-literal type 'long const[(1 << 30) << 2]'}}
     (1 << 30) << 2]{};   // expected-warning {{requires 34 bits to represent}}
 

clang/test/CXX/drs/dr3xx.cpp

@@ -898,8 +898,8 @@ namespace dr367 { // dr367: yes
   int c[true ? *new int : 4]; // expected-error 2{{variable length array}} expected-note {{read of uninitialized}}
   int d[true ? 4 : *new int];
 #if __cplusplus < 201103L
-  // expected-error@-4 {{variable length array}} expected-error@-4 {{constant expression}}
-  // expected-error@-3 {{variable length array}} expected-error@-3 {{constant expression}}
+  // expected-error@-4 2{{variable length array}}
+  // expected-error@-3 2{{variable length array}}
 #endif
 }
 

clang/test/CodeGen/vla.c

@@ -210,3 +210,15 @@ void test9(int n, int a[static n]) { }
 void test10(int a[static 0]) {}
 // NULL-INVALID: define void @test10(i32* nonnull align 4 %a)
 // NULL-VALID: define void @test10(i32* align 4 %a)
+
+const int constant = 32;
+// CHECK: define {{.*}}pr44406(
+int pr44406() {
+  int n = 0;
+  // Do not fold this VLA to an array of constant bound; that would miscompile
+  // this testcase.
+  char c[1][(constant - constant) + 3];
+  // CHECK: store i32 1,
+  sizeof(c[n = 1]);
+  return n;
+}

clang/test/Misc/warning-flags.c

@@ -91,4 +91,4 @@ CHECK-NEXT:   warn_weak_import
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 27
+CHECK: Number in -Wpedantic (not covered by other -W flags): 26

clang/test/PCH/cxx-constexpr.cpp

@@ -16,7 +16,7 @@ const int b = a;
 #else
 
 const int a = 5;
-typedef int T[b]; // expected-error {{variable length array}} expected-error {{must be an integer constant expression}} expected-note {{initializer of 'b'}}
+typedef int T[b]; // expected-error 2{{variable length array}} expected-note {{initializer of 'b'}}
 // expected-note@14 {{here}}
 typedef int T[5];
 

clang/test/Profile/misexpect-switch-default.c

@@ -10,7 +10,7 @@ void init_arry();
 
 const int inner_loop = 1000;
 const int outer_loop = 20;
-const int arry_size = 25;
+enum { arry_size = 25 };
 
 int arry[arry_size] = {0};
 

clang/test/Profile/misexpect-switch-nonconst.c

@@ -11,7 +11,7 @@ void init_arry();
 
 const int inner_loop = 1000;
 const int outer_loop = 20;
-const int arry_size = 25;
+enum { arry_size = 25 };
 
 int arry[arry_size] = {0};
 

clang/test/Profile/misexpect-switch-only-default-case.c

@@ -11,7 +11,7 @@ void init_arry();
 
 const int inner_loop = 1000;
 const int outer_loop = 20;
-const int arry_size = 25;
+enum { arry_size = 25 };
 
 int arry[arry_size] = {0};
 

clang/test/Profile/misexpect-switch.c

@@ -10,7 +10,7 @@ void init_arry();
 
 const int inner_loop = 1000;
 const int outer_loop = 20;
-const int arry_size = 25;
+enum { arry_size = 25 };
 
 int arry[arry_size] = {0};
 

clang/test/Sema/builtin-assume.c

@@ -23,7 +23,7 @@ int foo(int *a, int i) {
   __builtin_assume(ispure(i) > 2);
   __builtin_assume(ispure(++i) > 2); //expected-warning {{the argument to '__builtin_assume' has side effects that will be discarded}}
 
-  int test = sizeof(struct{char qq[(__builtin_assume(i != 5), 7)];});
+  int test = sizeof(struct{char qq[(__builtin_assume(i != 5), 7)];}); // expected-warning {{variable length array}}
 #endif
   return a[i];
 }

clang/test/Sema/builtins.c

@@ -141,7 +141,7 @@ typedef __typeof(sizeof(int)) size_t;
 size_t strlen(const char *);
 
 void test17() {
-#define ASSERT(...) { int arr[(__VA_ARGS__) ? 1 : -1]; }
+#define ASSERT(...) { enum { folded = (__VA_ARGS__) }; int arr[folded ? 1 : -1]; }
 #define T(...) ASSERT(__builtin_constant_p(__VA_ARGS__))
 #define F(...) ASSERT(!__builtin_constant_p(__VA_ARGS__))
 
@@ -179,12 +179,12 @@ void test17() {
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
   // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));        // expected-warning {{folded}}
-  ASSERT(!OPT(&test17_c[0]));    // expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));        // expected-warning {{folded}}
-  ASSERT(!OPT(&test17_d[0]));    // expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
+  ASSERT(!OPT(test17_c));        // expected-warning {{folding}}
+  ASSERT(!OPT(&test17_c[0]));    // expected-warning {{folding}}
+  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folding}}
+  ASSERT(!OPT(test17_d));        // expected-warning {{folding}}
+  ASSERT(!OPT(&test17_d[0]));    // expected-warning {{folding}}
+  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folding}}
 
 #undef OPT
 #undef T

clang/test/Sema/complex-int.c

@@ -60,14 +60,16 @@ void test5(_Complex int *x) {
   (*x)++;
 }
 
-int i1[(2+3i)*(5+7i) == 29i-11 ? 1 : -1];
-int i2[(29i-11)/(5+7i) == 2+3i ? 1 : -1];
-int i3[-(2+3i) == +(-3i-2) ? 1 : -1];
-int i4[~(2+3i) == 2-3i ? 1 : -1];
-int i5[(3i == -(-3i) ? ((void)3, 1i - 1) : 0) == 1i - 1 ? 1 : -1];
+// None of these array bounds is an ICE due to the use of literals of
+// non-integer type. But we can constant-fold all of them.
+int i1[(2+3i)*(5+7i) == 29i-11 ? 1 : -1]; // expected-warning {{fold}}
+int i2[(29i-11)/(5+7i) == 2+3i ? 1 : -1]; // expected-warning {{fold}}
+int i3[-(2+3i) == +(-3i-2) ? 1 : -1]; // expected-warning {{fold}}
+int i4[~(2+3i) == 2-3i ? 1 : -1]; // expected-warning {{fold}}
+int i5[(3i == -(-3i) ? ((void)3, 1i - 1) : 0) == 1i - 1 ? 1 : -1]; // expected-warning {{fold}}
 
-int f1[(2.0+3.0i)*(5.0+7.0i) == 29.0i-11.0 ? 1 : -1];
-int f2[(29.0i-11.0)/(5.0+7.0i) == 2.0+3.0i ? 1 : -1];
-int f3[-(2.0+3.0i) == +(-3.0i-2.0) ? 1 : -1];
-int f4[~(2.0+3.0i) == 2.0-3.0i ? 1 : -1];
-int f5[(3.0i == -(-3.0i) ? ((void)3.0, __extension__ (1.0i - 1.0)) : 0) == 1.0i - 1.0 ? 1 : -1];
+int f1[(2.0+3.0i)*(5.0+7.0i) == 29.0i-11.0 ? 1 : -1]; // expected-warning {{fold}}
+int f2[(29.0i-11.0)/(5.0+7.0i) == 2.0+3.0i ? 1 : -1]; // expected-warning {{fold}}
+int f3[-(2.0+3.0i) == +(-3.0i-2.0) ? 1 : -1]; // expected-warning {{fold}}
+int f4[~(2.0+3.0i) == 2.0-3.0i ? 1 : -1]; // expected-warning {{fold}}
+int f5[(3.0i == -(-3.0i) ? ((void)3.0, __extension__ (1.0i - 1.0)) : 0) == 1.0i - 1.0 ? 1 : -1]; // expected-warning {{fold}}

clang/test/Sema/const-eval-64.c

@@ -1,8 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s
-// expected-no-diagnostics
 
 #define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];});
 
 // <rdar://problem/10962435>
-EVAL_EXPR(1, ((char*)-1LL) + 1 == 0 ? 1 : -1)
-EVAL_EXPR(2, ((char*)-1LL) + 1 < (char*) -1 ? 1 : -1)
+EVAL_EXPR(1, ((char*)-1LL) + 1 == 0 ? 1 : -1) // expected-warning {{folded}}
+EVAL_EXPR(2, ((char*)-1LL) + 1 < (char*) -1 ? 1 : -1) // expected-warning {{folded}}

clang/test/Sema/const-eval.c

@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s -Wno-tautological-pointer-compare -Wno-pointer-to-int-cast
 
-#define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];});
+#define EVAL_EXPR(testno, expr) enum { test##testno = (expr) }; struct check_positive##testno { int a[test##testno]; };
 int x;
 EVAL_EXPR(1, (_Bool)&x)
 EVAL_EXPR(2, (int)(1.0+(double)4))
@@ -14,12 +14,12 @@ EVAL_EXPR(8, (_Bool)"asdf")
 EVAL_EXPR(9, !!&x)
 EVAL_EXPR(10, ((void)1, 12))
 void g0(void);
-EVAL_EXPR(11, (g0(), 12)) // expected-error {{must have a constant size}}
+EVAL_EXPR(11, (g0(), 12)) // expected-error {{not an integer constant expression}}
 EVAL_EXPR(12, 1.0&&2.0)
-EVAL_EXPR(13, x || 3.0) // expected-error {{must have a constant size}}
+EVAL_EXPR(13, x || 3.0) // expected-error {{not an integer constant expression}}
 
 unsigned int l_19 = 1;
-EVAL_EXPR(14, (1 ^ l_19) && 1); // expected-error {{fields must have a constant size}}
+EVAL_EXPR(14, (1 ^ l_19) && 1); // expected-error {{not an integer constant expression}}
 
 void f()
 {
@@ -36,7 +36,7 @@ int g17[(3?:1) - 2];
 EVAL_EXPR(18, ((int)((void*)10 + 10)) == 20 ? 1 : -1);
 
 struct s {
-  int a[(int)-1.0f]; // expected-error {{'a' declared as an array with a negative size}}
+  int a[(int)-1.0f]; // expected-error {{array size is negative}}
 };
 
 EVAL_EXPR(19, ((int)&*(char*)10 == 10 ? 1 : -1));
@@ -47,9 +47,9 @@ EVAL_EXPR(21, (__imag__ 2i) == 2 ? 1 : -1);
 
 EVAL_EXPR(22, (__real__ (2i+3)) == 3 ? 1 : -1);
 
-int g23[(int)(1.0 / 1.0)] = { 1 };
-int g24[(int)(1.0 / 1.0)] = { 1 , 2 }; // expected-warning {{excess elements in array initializer}}
-int g25[(int)(1.0 + 1.0)], g26 = sizeof(g25);
+int g23[(int)(1.0 / 1.0)] = { 1 }; // expected-warning {{folded to constant array}}
+int g24[(int)(1.0 / 1.0)] = { 1 , 2 }; // expected-warning {{folded to constant array}} expected-warning {{excess elements in array initializer}}
+int g25[(int)(1.0 + 1.0)], g26 = sizeof(g25); // expected-warning {{folded to constant array}}
 
 EVAL_EXPR(26, (_Complex double)0 ? -1 : 1)
 EVAL_EXPR(27, (_Complex int)0 ? -1 : 1)
@@ -116,17 +116,17 @@ EVAL_EXPR(42, __builtin_constant_p(pr11391.f = 1))
 // PR12043
 float varfloat;
 const float constfloat = 0;
-EVAL_EXPR(43, varfloat && constfloat) // expected-error {{must have a constant size}}
+EVAL_EXPR(43, varfloat && constfloat) // expected-error {{not an integer constant expression}}
 
 // <rdar://problem/10962435>
 EVAL_EXPR(45, ((char*)-1) + 1 == 0 ? 1 : -1)
 EVAL_EXPR(46, ((char*)-1) + 1 < (char*) -1 ? 1 : -1)
 EVAL_EXPR(47, &x < &x + 1 ? 1 : -1)
 EVAL_EXPR(48, &x != &x - 1 ? 1 : -1)
-EVAL_EXPR(49, &x < &x - 100 ? 1 : -1) // expected-error {{must have a constant size}}
+EVAL_EXPR(49, &x < &x - 100 ? 1 : -1) // expected-error {{not an integer constant expression}}
 
 extern struct Test50S Test50;
-EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned long)&Test50 + 10)) // expected-error {{must have a constant size}}
+EVAL_EXPR(50, &Test50 < (struct Test50S*)((unsigned long)&Test50 + 10)) // expected-error {{not an integer constant expression}}
 
 // <rdar://problem/11874571>
 EVAL_EXPR(51, 0 != (float)1e99)
@@ -136,7 +136,7 @@ void PR21945() { int i = (({}), 0l); }
 
 void PR24622();
 struct PR24622 {} pr24622;
-EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{must have a constant size}}
+EVAL_EXPR(52, &pr24622 == (void *)&PR24622); // expected-error {{not an integer constant expression}}
 
 // We evaluate these by providing 2s' complement semantics in constant
 // expressions, like we do for integers.

clang/test/Sema/darwin-align-cast.c

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
-// expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;
 typedef __darwin_size_t size_t;
@@ -18,6 +17,7 @@ i386/_param.h:#define __DARWIN_ALIGN(p) ((__darwin_size_t)((char *)(p)
 
 ssize_t sendFileDescriptor(int fd, void *data, size_t nbytes, int sendfd) {
   union {
+    // expected-warning@+1 {{folded to constant array}}
     char control[(((__darwin_size_t)((char *)(sizeof(struct cmsghdr)) + (sizeof(__darwin_size_t) - 1)) &~ (sizeof(__darwin_size_t) - 1)) + ((__darwin_size_t)((char *)(sizeof(int)) + (sizeof(__darwin_size_t) - 1)) &~ (sizeof(__darwin_size_t) - 1)))];
   } control_un;
   return 0;