Re-fix r51907 in a way which doesn't affect valid code. This essentially

moves the check for the invalid construct to a point where it doesn't 
affect other uses of isIntegerConstantExpr, and we can warn properly 
when the extension is used.  This makes it a bit more complicated, but 
it's a lot cleaner.

Steve, please tell me if this check is sufficient to handle the 
relevant system header.  I know it's enough to handle the testcase, but 
I don't know what exactly the original looks like.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@51918 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 62 insertions, 8 deletions

diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def
index 5f691c97d4..7b88fce2ff 100644
--- a/include/clang/Basic/DiagnosticKinds.def
+++ b/include/clang/Basic/DiagnosticKinds.def
@@ -721,6 +721,8 @@ DIAG(warn_enum_too_large, WARNING,
      "enumeration values exceed range of largest integer")
 DIAG(err_case_label_not_integer_constant_expr, ERROR,
      "case label does not reduce to an integer constant")
+DIAG(warn_illegal_constant_array_size, EXTENSION,
+     "size of static array must be an integer constant expression")
 DIAG(err_typecheck_illegal_vla, ERROR,
      "arrays with static storage duration must have constant integer length")
 DIAG(err_typecheck_negative_array_size, ERROR,
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index a89fbd621a..357d711978 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -918,11 +918,6 @@ bool Expr::isIntegerConstantExpr(llvm::APSInt &Result, ASTContext &Ctx,
     if (!SubExpr->getType()->isArithmeticType() ||
         !getType()->isIntegerType()) {
       if (Loc) *Loc = SubExpr->getLocStart();
-      // GCC accepts pointers as an extension.
-      // FIXME: check getLangOptions().NoExtensions. At the moment, it doesn't
-      // appear possible to get langOptions() from the Expr.
-      if (SubExpr->getType()->isPointerType()) // && !NoExtensions
-        return true;
       return false;
     }
 
diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 5d9113e99d..a874fa18c1 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -896,6 +896,11 @@ private:
 
   
   void InitBuiltinVaListType();
+
+  // Helper method to turn variable array types into
+  // constant array types in certain situations which would otherwise
+  // be errors
+  QualType TryFixInvalidVariablyModifiedType(QualType T);
   
   //===--------------------------------------------------------------------===//
   // Extra semantic analysis beyond the C type system
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 230ea3cb9d..c5050d201b 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1842,6 +1842,52 @@ Sema::DeclTy *Sema::ActOnTag(Scope *S, unsigned TagType, TagKind TK,
   return New;
 }
 
+static bool CalcFakeICEVal(const Expr* Expr,
+                           llvm::APSInt& Result,
+                           ASTContext& Context) {
+  // Calculate the value of an expression that has a calculatable
+  // value, but isn't an ICE. Currently, this only supports
+  // a very narrow set of extensions, but it can be expanded if needed.
+  if (const ParenExpr *PE = dyn_cast<ParenExpr>(Expr))
+    return CalcFakeICEVal(PE->getSubExpr(), Result, Context);
+
+  if (const CastExpr *CE = dyn_cast<CastExpr>(Expr)) {
+    QualType CETy = CE->getType();
+    if ((CETy->isIntegralType() && !CETy->isBooleanType()) ||
+        CETy->isPointerType()) {
+      if (CalcFakeICEVal(CE->getSubExpr(), Result, Context)) {
+        Result.extOrTrunc(Context.getTypeSize(CETy));
+        // FIXME: This assumes pointers are signed.
+        Result.setIsSigned(CETy->isSignedIntegerType() ||
+                           CETy->isPointerType());
+        return true;
+      }
+    }
+  }
+
+  if (Expr->getType()->isIntegralType())
+    return Expr->isIntegerConstantExpr(Result, Context);
+
+  return false;
+}
+
+QualType Sema::TryFixInvalidVariablyModifiedType(QualType T) {
+  // This method tries to turn a variable array into a constant
+  // array even when the size isn't an ICE.  This is necessary
+  // for compatibility with code that depends on gcc's buggy
+  // constant expression folding, like struct {char x[(int)(char*)2];}
+  if (const VariableArrayType* VLATy = dyn_cast<VariableArrayType>(T)) {
+    llvm::APSInt Result(32);
+    if (VLATy->getSizeExpr() &&
+        CalcFakeICEVal(VLATy->getSizeExpr(), Result, Context) &&
+        Result > llvm::APSInt(Result.getBitWidth(), Result.isUnsigned())) {
+      return Context.getConstantArrayType(VLATy->getElementType(),
+                                          Result, ArrayType::Normal, 0);
+    }
+  }
+  return QualType();
+}
+
 /// ActOnField - Each field of a struct/union/class is passed into this in order
 /// to create a FieldDecl object for it.
 Sema::DeclTy *Sema::ActOnField(Scope *S,
@@ -1877,9 +1923,15 @@ Sema::DeclTy *Sema::ActOnField(Scope *S,
   // C99 6.7.2.1p8: A member of a structure or union may have any type other
   // than a variably modified type.
   if (T->isVariablyModifiedType()) {
-    // FIXME: This diagnostic needs work
-    Diag(Loc, diag::err_typecheck_illegal_vla, Loc);
-    InvalidDecl = true;
+    QualType FixedTy = TryFixInvalidVariablyModifiedType(T);
+    if (!FixedTy.isNull()) {
+      Diag(Loc, diag::warn_illegal_constant_array_size, Loc);
+      T = FixedTy;
+    } else {
+      // FIXME: This diagnostic needs work
+      Diag(Loc, diag::err_typecheck_illegal_vla, Loc);
+      InvalidDecl = true;
+    }
   }
   // FIXME: Chain fielddecls together.
   FieldDecl *NewFD = FieldDecl::Create(Context, Loc, II, T, BitWidth);