"Enhance" CheckArithmeticConstantExpression to accept ?: with a constant

condition as a constant even if the unevaluated side is a not a constant.
We don't do this when extensions are off, and we emit a warning when this
happens:

t.c:22:11: warning: expression is not a constant, but is accepted as one by GNU extensions
short t = __builtin_constant_p(5353) ? 42 : somefunc();
          ^                                 ~~~~~~~~~~

suggestions for improvement are welcome.  This is obviously horrible, but
is required for real-world code.

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

diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def
index 03acd3c9801605f1703616c387e3bb9dd83235fd..1d62c8d64cfad2113e7fa28a6dddbe14502df0ad 100644 (file)
--- a/include/clang/Basic/DiagnosticKinds.def
+++ b/include/clang/Basic/DiagnosticKinds.def
@@ -1096,6 +1096,8 @@ DIAG(warn_typecheck_cond_incompatible_pointers, WARNING,
      "pointer type mismatch ('%0' and '%1')")
 DIAG(err_typecheck_choose_expr_requires_constant, ERROR,
      "'__builtin_choose_expr' requires a constant expression")
+DIAG(ext_typecheck_expression_not_constant_but_accepted, EXTENSION,
+     "expression is not a constant, but is accepted as one by GNU extensions")
 DIAG(warn_unused_expr, WARNING,
      "expression result unused")
 DIAG(err_pascal_string_too_long, ERROR,

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index e1215a5e9e4fa57f6e4cea05e49aaca96b483bf8..74ce855add5baecb277f541dfe7f139d7163ed8f 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1190,12 +1190,47 @@ bool Sema::CheckArithmeticConstantExpression(const Expr* Init) {
   }
   case Expr::ConditionalOperatorClass: {
     const ConditionalOperator *Exp = cast<ConditionalOperator>(Init);
-    if (CheckArithmeticConstantExpression(Exp->getCond()))
-      return true;
-    if (Exp->getLHS() &&
-        CheckArithmeticConstantExpression(Exp->getLHS()))
+    
+    // If GNU extensions are disabled, we require all operands to be arithmetic
+    // constant expressions.
+    if (getLangOptions().NoExtensions) {
+      return CheckArithmeticConstantExpression(Exp->getCond()) ||
+          (Exp->getLHS() && CheckArithmeticConstantExpression(Exp->getLHS())) ||
+             CheckArithmeticConstantExpression(Exp->getRHS());
+    }
+
+    // Otherwise, we have to emulate some of the behavior of fold here.
+    // Basically GCC treats things like "4 ? 1 : somefunc()" as a constant
+    // because it can constant fold things away.  To retain compatibility with
+    // GCC code, we see if we can fold the condition to a constant (which we
+    // should always be able to do in theory).  If so, we only require the
+    // specified arm of the conditional to be a constant.  This is a horrible
+    // hack, but is require by real world code that uses __builtin_constant_p.
+    APValue Val;
+    if (!Exp->getCond()->tryEvaluate(Val, Context)) {
+      // If the tryEvaluate couldn't fold it, CheckArithmeticConstantExpression
+      // won't be able to either.  Use it to emit the diagnostic though.
+      bool Res = CheckArithmeticConstantExpression(Exp->getCond());
+      assert(Res && "tryEvaluate couldn't evaluate this constant?");
+      return Res;
+    }
+    
+    // Verify that the side following the condition is also a constant.
+    const Expr *TrueSide = Exp->getLHS(), *FalseSide = Exp->getRHS();
+    if (Val.getInt() == 0) 
+      std::swap(TrueSide, FalseSide);
+    
+    if (TrueSide && CheckArithmeticConstantExpression(TrueSide))
       return true;
-    return CheckArithmeticConstantExpression(Exp->getRHS());
+      
+    // Okay, the evaluated side evaluates to a constant, so we accept this.
+    // Check to see if the other side is obviously not a constant.  If so, 
+    // emit a warning that this is a GNU extension.
+    if (FalseSide && !FalseSide->tryEvaluate(Val, Context))
+      Diag(Init->getExprLoc(), 
+           diag::ext_typecheck_expression_not_constant_but_accepted,
+           FalseSide->getSourceRange());
+    return false;
   }
   }
 }
@@ -1211,20 +1246,8 @@ bool Sema::CheckForConstantInitializer(Expr *Init, QualType DclT) {
     return CheckForConstantInitializer(e->getInitializer(), DclT);
 
   if (Init->getType()->isReferenceType()) {
-    // FIXME: Work out how the heck reference types work
+    // FIXME: Work out how the heck references work.
     return false;
-#if 0
-    // A reference is constant if the address of the expression
-    // is constant
-    // We look through initlists here to simplify
-    // CheckAddressConstantExpressionLValue.
-    if (InitListExpr *Exp = dyn_cast<InitListExpr>(Init)) {
-      assert(Exp->getNumInits() > 0 &&
-             "Refernce initializer cannot be empty");
-      Init = Exp->getInit(0);
-    }
-    return CheckAddressConstantExpressionLValue(Init);
-#endif
   }
 
   if (InitListExpr *Exp = dyn_cast<InitListExpr>(Init)) {

diff --git a/test/Sema/constant-builtins.c b/test/Sema/constant-builtins.c
index 875414a109e63a3176576d16a68e8bebaaf54c7a..d6cf45755daa19d8c1ba59b9cfd81be5b142ea8d 100644 (file)
--- a/test/Sema/constant-builtins.c
+++ b/test/Sema/constant-builtins.c
@@ -1,4 +1,4 @@
-// RUN: clang -fsyntax-only %s
+// RUN: clang -fsyntax-only %s -verify -pedantic
 
 // Math stuff
 
@@ -13,6 +13,11 @@ long double  g5 = __builtin_infl();
 
 extern int f();
 
-int h0 = __builtin_types_compatible_p(int,float);
+int h0 = __builtin_types_compatible_p(int,float); // expected-warning {{extension}}
 //int h1 = __builtin_choose_expr(1, 10, f());
 //int h2 = __builtin_expect(0, 0);
+
+short somefunc();
+
+short t = __builtin_constant_p(5353) ? 42 : somefunc(); // expected-warning {{expression is not a constant, but is accepted as one by GNU extensions}}
+