[cfe-commits] r51918 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/AST/Expr.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp

[Eli Friedman]

Author: efriedma
Date: Tue Jun  3 16:01:11 2008
New Revision: 51918

URL: http://llvm.org/viewvc/llvm-project?rev=51918&view=rev
Log:
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.


Modified:
    cfe/trunk/include/clang/Basic/DiagnosticKinds.def
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=51918&r1=51917&r2=51918&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Tue Jun  3 16:01:11 2008
@@ -721,6 +721,8 @@
      "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,

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=51918&r1=51917&r2=51918&view=diff

==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Jun  3 16:01:11 2008
@@ -918,11 +918,6 @@
     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;
     }
 

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=51918&r1=51917&r2=51918&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Tue Jun  3 16:01:11 2008
@@ -896,6 +896,11 @@
 
   
   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

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=51918&r1=51917&r2=51918&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun  3 16:01:11 2008
@@ -1842,6 +1842,52 @@
   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 @@
   // 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);