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

diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def
index 5f691c97d4841ba975eeb68ec62489ee6a98293c..7b88fce2ff6c4959b08b7bfc637432254e5f9747 100644 (file)
--- 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 a89fbd621a5632b8ca733156f5f59775c1db8c3a..357d711978b3784ae64d8bc586176022234e5b32 100644 (file)
--- 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 5d9113e99df879497018fc61950c98a14520295b..a874fa18c15eabf67af12f142517cd4a1342970f 100644 (file)
--- 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 230ea3cb9da72d6fec8d0faebcf7973b4ba0eb4a..c5050d201b1b4fe27ad1926e67a0009d0d15e411 100644 (file)
--- 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);