Use is.constant intrinsic for __builtin_constant_p

[void]

Summary

A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if it's a variable that's in a context where
it may resolve to a constant, e.g., an argument to a function after
inlining.

[void]

Adding ConstantExpr visitor.

[void]

This has D54356 integrated into it. D54356 should be reviewed and submitted first, even though it's out of order.

[void]

Ping?

[rsmith]

include/clang/AST/Expr.h

908–912

If we're creating two ConstantExpr wrappers for the same expression, that seems like a bug in the caller to me. When does this happen?

lib/AST/Expr.cpp

2924–2925

Can we just return true here? If not, please add a FIXME saying that we should be able to.

lib/Sema/SemaExpr.cpp

4991–4992

We should create this node as part of checking that the argument is an ICE (in SemaBuiltinConstantArg).

14171–14172

I don't understand why CurContext matters here. Can you explain the intent of this check?

[void]

Remove some extraneous checks.

[void]

include/clang/AST/Expr.h

908–912

I saw a place that was trying to do that, but it may no longer be valid. I can remove the check for now.

lib/AST/Expr.cpp

2924–2925

To be honest, I don't know. Theoretically we should be able to. Let me give it a try and see how it goes.

lib/Sema/SemaExpr.cpp

14171–14172

It may have been an artifact of development. I can remove it.

[void]

lib/AST/Expr.cpp

2924–2925

I tested this and it looks like it could lead to an extra warning like this:

File /sandbox/morbo/llvm/llvm-mirror/tools/clang/test/Sema/array-init.c Line 285: cannot initialize array of type 'int [5]' with non-constant array of type 'int [5]'

(Similar in Sema/compound-literal.c.) I'll add a comment.

lib/Sema/SemaExpr.cpp

4991–4992

It turns out that SemaBuiltinConstantArg isn't called for __builtin_constant_p(). Its argument type is ., which...I'm not sure what that means. It looks like a vararg, but that doesn't make sense for the builtin.

Should I change its signature in Builtins.def?

[rsmith]

lib/CodeGen/CGBuiltin.cpp

1931–1932

Are there cases where a call to an always_inline function could change the outcome?

lib/Sema/SemaExpr.cpp

4991–4992

It's also marked t, which means "signature is meaningless, uses custom type checking".

The argument to __builtin_constant_p isn't required to be an ICE, though, so we shouldn't be calling SemaBuiltinConstantArg for it / creating a ConstantExpr wrapping it, as far as I can tell.

[void]

Updated to address comments.

[void]

I think this is ready now. PTAL.

lib/CodeGen/CGBuiltin.cpp

1931–1932

Yes, but the result of bcp isn't guaranteed to be the same for optimized and unoptimized code. GCC's documentation says that it won't return 1 unless -O is used. Their code appears to back this up somewhat:

switch (fcode)
  {
  case BUILT_IN_CONSTANT_P:
    {
      tree val = fold_builtin_constant_p (arg0);

      /* Gimplification will pull the CALL_EXPR for the builtin out of
         an if condition.  When not optimizing, we'll not CSE it back.
         To avoid link error types of regressions, return false now.  */
      if (!val && !optimize)
        val = integer_zero_node;

      return val;
    }
  ...
  }

Our inlining is different of course, and I'm not sure when GCC performs their always_inline inlining. It may be before the above code is called. But I think we're within reasonable bounds with respect to the documentation.

(That said, GCC has a tendency to violate its own documentation. So caveat emptor, etc.)

lib/Sema/SemaExpr.cpp

4991–4992

Right. I think I was looking at something else. I changed it.

[void]

Fix overflow flag.

[void]

ImpCast.

[void]

Ping? :-)

[void]

Ping? I really don't want this review to go on forever.

[void]

Don't re-wrap a ConstExpr.

[void]

No function pointers

[rsmith]

Looks good other than the ODR violation issue.

include/clang/AST/Expr.h

37–39

It's not appropriate to declare internal-linkage types in headers. This will create ODR violations, and appears to be unnecessary -- the extra EvaluateAsRValue overload you declare below is not callable outside of ExprConstant.cpp, so can instead be declared as an internal linkage function in that file.

[void]

Remove ODR violation.

[void]

Should be okay now. PTAL. :-)

[nickdesaulniers]

lib/StaticAnalyzer/Core/ExprEngine.cpp

1285–1286

LLVM_FALLTHROUGH; ?

[nickdesaulniers]

lib/StaticAnalyzer/Core/ExprEngine.cpp

1285–1286

nvm, only needed when there's code THEN fallthrough.

[uweigand]

It seems this patch caused the SystemZ build bots to fail, they're now all running into assertion failures:

ICE cannot be evaluated!
UNREACHABLE executed at /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/lib/AST/ExprConstant.cpp:11442!

See e.g. http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/20645/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Abuiltins-systemz-zvector-error.c

[nickdesaulniers]

lib/AST/ExprConstant.cpp

11391

@void , I assume this unreachable is the one reported by @uweigand ? Does the above switch need a case statement added?

[MaskRay]

https://reviews.llvm.org/rC347417 makes constexpr string_view service = "HELLO WORD SERVICE" (P0426) broken with libstdc++

% cat a.cc
constexpr bool __constant_string_p(const char *__s) {
  while (__builtin_constant_p(*__s) && *__s)
    __s++;
  return __builtin_constant_p(*__s);
}

constexpr bool n = __constant_string_p("a");

% fclang++ -std=c++17 -fsyntax-only a.cc
a.cc:1:16: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
constexpr bool __constant_string_p(const char *__s) {
               ^
a.cc:2:10: note: subexpression not valid in a constant expression
  while (__builtin_constant_p(*__s) && *__s)
         ^
a.cc:7:16: error: constexpr variable 'n' must be initialized by a constant expression
constexpr bool n = __constant_string_p("a");
               ^   ~~~~~~~~~~~~~~~~~~~~~~~~
a.cc:2:10: note: subexpression not valid in a constant expression
  while (__builtin_constant_p(*__s) && *__s)
         ^
a.cc:7:20: note: in call to '__constant_string_p(&"a"[0])'
constexpr bool n = __constant_string_p("a");
                   ^
2 errors generated.

Fourth time should be a charm...

[void]

Doh! I'll take a look at it.

[void]

Fixed in r347531.

[craig.topper]

I'm seeing a case where an inline assembly 'n' constraint is behaving differently in -O0 now. Previously we would evaluate the builtin_constant_p as part of the EvaluateAsInt call in CodeGenFunction::EmitAsmInput, but I think we're not doing that now. This causes us to fail later because the constraint wasn't satisfied since we were in -O0 and thus we didn't optimize the rest of the expression. The test case we have is a nasty mix of builtin_constant_p, builtin_choose_expr, and builtin_classify_type. I can see about sharing it if needed.

[void]

In D54355#1326494, @craig.topper wrote:

I'm seeing a case where an inline assembly 'n' constraint is behaving differently in -O0 now. Previously we would evaluate the builtin_constant_p as part of the EvaluateAsInt call in CodeGenFunction::EmitAsmInput, but I think we're not doing that now. This causes us to fail later because the constraint wasn't satisfied since we were in -O0 and thus we didn't optimize the rest of the expression. The test case we have is a nasty mix of builtin_constant_p, builtin_choose_expr, and builtin_classify_type. I can see about sharing it if needed.

__builtin_constant_p() is known to have different semantics at -O0 than it does during optimizations. That said, we shouldn't be intentionally generating incorrect code. Could you test it against gcc to see if it has the same semantics? Otherwise, please add a testcase. :-)

[craig.topper]

Here's the test case that we have https://reviews.llvm.org/P8123 gcc seems to accept it at O0

[jyknight]

In D54355#1327237, @craig.topper wrote:

Here's the test case that we have https://reviews.llvm.org/P8123 gcc seems to accept it at O0

Smaller test case:

extern unsigned long long __sdt_unsp;

void foo() {
  __asm__ __volatile__(
      "" :: "n"( (__builtin_constant_p(__sdt_unsp) || 0) ? 1 : -1));
}

[void]

The issue is that "n" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of __builtin_constant_p shouldn't be delayed (it being compiled at -O0). I suspect that this issue will also happen with the "i" constraint.

[jyknight]

In D54355#1328557, @void wrote:

The issue is that "n" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of __builtin_constant_p shouldn't be delayed (it being compiled at -O0). I suspect that this issue will also happen with the "i" constraint.

Indeed. We _do_ actually already know that in clang too, however -- clang already knows that "n" requires an immediate, and does constant expression evaluation for it, e.g. to expand constexpr functions. Grep for "requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p correctly, though.

(Note, as a workaround, you could use | instead of ||, or put the expression as the value of an enumeration constant).

[void]

In D54355#1328786, @jyknight wrote:

In D54355#1328557, @void wrote:

The issue is that "n" is expecting an immediate value, but the result of the ternary operator isn't calculated by the front-end, because it doesn't "know" that the evaluation of __builtin_constant_p shouldn't be delayed (it being compiled at -O0). I suspect that this issue will also happen with the "i" constraint.

Indeed. We _do_ actually already know that in clang too, however -- clang already knows that "n" requires an immediate, and does constant expression evaluation for it, e.g. to expand constexpr functions. Grep for "requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p correctly, though.

(Note, as a workaround, you could use | instead of ||, or put the expression as the value of an enumeration constant).

I looked at requiresImmediateConstant, but when I set it for n and i things broke all over the place. I sent out a patch for review. It's limited to -O0, since we can compile with optimizations without complaint. And there may be instances where it's necessary:

inline void foo(int x) {
  asm("" : : "n" (__builtin_constant_p(n) ? 1 : -1));
}

void bar() {
  foo(37);
}

[jyknight]

That example cannot be expected to ever evaluate the expression as "1" -- it doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but not, e.g., "nr") must require a constant expression, and evaluating the argument as a constant expression necessarily means always evaluating it to -1.

include/clang/AST/Expr.h

 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/TrailingObjects.h"
 
 namespace clang {
   class APValue;
   class ASTContext;
   class BlockDecl;
   class CXXBaseSpecifier;
   class CXXMemberCallExpr;
   class CXXOperatorCallExpr;
   class CastExpr;
   class Decl;
   class IdentifierInfo;
   class MaterializeTemporaryExpr;
public:
   static bool classof(const Stmt *T) {
     return T->getStmtClass() >= firstFullExprConstant &&
            T->getStmtClass() <= lastFullExprConstant;
   }
 };
 
 /// ConstantExpr - An expression that occurs in a constant context.
 class ConstantExpr : public FullExpr {
-public:
   ConstantExpr(Expr *subexpr)
     : FullExpr(ConstantExprClass, subexpr) {}
 
+public:
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+    assert(!isa<ConstantExpr>(E));
+    return new (Context) ConstantExpr(E);
+  }
+
   /// Build an empty constant expression wrapper.
   explicit ConstantExpr(EmptyShell Empty)
     : FullExpr(ConstantExprClass, Empty) {}
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
     return SubExpr->getBeginLoc();
   }
   SourceLocation getEndLoc() const LLVM_READONLY {
public:
   friend class CastExpr;
 };
 
 inline Expr *Expr::IgnoreImpCasts() {
   Expr *e = this;
   while (true)
     if (ImplicitCastExpr *ice = dyn_cast<ImplicitCastExpr>(e))
       e = ice->getSubExpr();
-    else if (ConstantExpr *ce = dyn_cast<ConstantExpr>(e))
-      e = ce->getSubExpr();
+    else if (FullExpr *fe = dyn_cast<FullExpr>(e))
+      e = fe->getSubExpr();
     else
       break;
   return e;
 }
 
 /// ExplicitCastExpr - An explicit cast written in the source
 /// code.
 ///

lib/AST/ASTImporter.cpp

 ExpectedStmt ASTNodeImporter::VisitConstantExpr(ConstantExpr *E) {
   auto Imp = importSeq(E->getSubExpr());
   if (!Imp)
     return Imp.takeError();
 
   Expr *ToSubExpr;
   std::tie(ToSubExpr) = *Imp;
 
-  return new (Importer.getToContext()) ConstantExpr(ToSubExpr);
+  return ConstantExpr::Create(Importer.getToContext(), ToSubExpr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitParenExpr(ParenExpr *E) {
   auto Imp = importSeq(E->getLParen(), E->getRParen(), E->getSubExpr());
   if (!Imp)
     return Imp.takeError();
 
   SourceLocation ToLParen, ToRParen;

lib/AST/Expr.cpp

if (MaterializeTemporaryExpr *Materialize
       E = Materialize->GetTemporaryExpr();
       continue;
     }
     if (SubstNonTypeTemplateParmExpr *NTTP
                                   = dyn_cast<SubstNonTypeTemplateParmExpr>(E)) {
       E = NTTP->getReplacement();
       continue;
     }
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E)) {
-      E = CE->getSubExpr();
+    if (FullExpr *FE = dyn_cast<FullExpr>(E)) {
+      E = FE->getSubExpr();
       continue;
     }
     return E;
   }
 }
 
 Expr *Expr::IgnoreCasts() {
   Expr *E = this;
   while (true) {
     if (CastExpr *P = dyn_cast<CastExpr>(E)) {
       E = P->getSubExpr();
       continue;
     }
     if (MaterializeTemporaryExpr *Materialize
         = dyn_cast<MaterializeTemporaryExpr>(E)) {
       E = Materialize->GetTemporaryExpr();
       continue;
     }
     if (SubstNonTypeTemplateParmExpr *NTTP
         = dyn_cast<SubstNonTypeTemplateParmExpr>(E)) {
       E = NTTP->getReplacement();
       continue;
     }
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E)) {
-      E = CE->getSubExpr();
+    if (FullExpr *FE = dyn_cast<FullExpr>(E)) {
+      E = FE->getSubExpr();
       continue;
     }
     return E;
   }
 }
 
 /// IgnoreParenLValueCasts - Ignore parentheses and lvalue-to-rvalue
 /// casts.  This is intended purely as a temporary workaround for code
while (true) {
     } else if (MaterializeTemporaryExpr *Materialize
                                       = dyn_cast<MaterializeTemporaryExpr>(E)) {
       E = Materialize->GetTemporaryExpr();
       continue;
     } else if (SubstNonTypeTemplateParmExpr *NTTP
                                   = dyn_cast<SubstNonTypeTemplateParmExpr>(E)) {
       E = NTTP->getReplacement();
       continue;
-    } else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E)) {
-      E = CE->getSubExpr();
+    } else if (FullExpr *FE = dyn_cast<FullExpr>(E)) {
+      E = FE->getSubExpr();
       continue;
     }
     break;
   }
   return E;
 }
 
 Expr *Expr::ignoreParenBaseCasts() {
if (CE->getConstructor()->isTrivial() &&
 
       // Trivial copy constructor
       assert(CE->getNumArgs() == 1 && "trivial ctor with > 1 argument");
       return CE->getArg(0)->isConstantInitializer(Ctx, false, Culprit);
     }
 
     break;
   }
+  case ConstantExprClass: {
+    // FIXME: We should be able to return "true" here, but it can lead to extra
+    // error messages. E.g. in Sema/array-init.c.
+    const Expr *Exp = cast<ConstantExpr>(this)->getSubExpr();
+    return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }
   case CompoundLiteralExprClass: {
     // This handles gcc's extension that allows global initializers like
     // "struct x {int x;} x = (struct x) {};".
     // FIXME: This accepts other cases it shouldn't!
     const Expr *Exp = cast<CompoundLiteralExpr>(this)->getInitializer();
     return Exp->isConstantInitializer(Ctx, false, Culprit);
   }
   case DesignatedInitUpdateExprClass: {

lib/AST/ExprConstant.cpp

 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/OSLog.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstring>
 #include <functional>
 
 #define DEBUG_TYPE "exprconstant"
 
 using namespace clang;
 using llvm::APSInt;
struct EvalInfo {
 
     /// Have we emitted a diagnostic explaining why we couldn't constant
     /// fold (not just why it's not strictly a constant expression)?
     bool HasFoldFailureDiagnostic;
 
     /// Whether or not we're currently speculatively evaluating.
     bool IsSpeculativelyEvaluating;
 
+    /// Whether or not we're in a context where the front end requires a
+    /// constant value.
+    bool InConstantContext;
+
     enum EvaluationMode {
       /// Evaluate as a constant expression. Stop if we find that the expression
       /// is not a constant expression.
       EM_ConstantExpression,
 
       /// Evaluate as a potential constant expression. Keep going if we hit a
       /// construct that we can't evaluate yet (because we don't yet know the
       /// value of something) but stop if we hit something that could never be
struct EvalInfo {
     EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
       : Ctx(const_cast<ASTContext &>(C)), EvalStatus(S), CurrentCall(nullptr),
         CallStackDepth(0), NextCallIndex(1),
         StepsLeft(getLangOpts().ConstexprStepLimit),
         BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
         EvaluatingDecl((const ValueDecl *)nullptr),
         EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
         HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-        EvalMode(Mode) {}
+        InConstantContext(false), EvalMode(Mode) {}
 
     void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
       EvaluatingDecl = Base;
       EvaluatingDeclValue = &Value;
       EvaluatingConstructors.insert({Base, {0, 0}});
     }
 
     const LangOptions &getLangOpts() const { return Ctx.getLangOpts(); }
public:
   }
 
   bool ZeroInitialization(const Expr *E) { return Success(0, E); }
 
   //===--------------------------------------------------------------------===//
   //                            Visitor Methods
   //===--------------------------------------------------------------------===//
 
+  bool VisitConstantExpr(const ConstantExpr *E);
+
   bool VisitIntegerLiteral(const IntegerLiteral *E) {
     return Success(E->getValue(), E);
   }
   bool VisitCharacterLiteral(const CharacterLiteral *E) {
     return Success(E->getValue(), E);
   }
 
   bool CheckReferencedDecl(const Expr *E, const Decl *D);
static bool tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type,
   // write to/read from.
   if (EndOffset <= LVal.getLValueOffset())
     Size = 0;
   else
     Size = (EndOffset - LVal.getLValueOffset()).getQuantity();
   return true;
 }
 
+bool IntExprEvaluator::VisitConstantExpr(const ConstantExpr *E) {
+  llvm::SaveAndRestore<bool> InConstantContext(Info.InConstantContext, true);
+  return ExprEvaluatorBaseTy::VisitConstantExpr(E);
+}
+
 bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) {
   if (unsigned BuiltinOp = E->getBuiltinCallee())
     return VisitBuiltinCallExpr(E, BuiltinOp);
 
   return ExprEvaluatorBaseTy::VisitCallExpr(E);
 }
 
 bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
case Builtin::BI__builtin_clzs: {
     if (!EvaluateInteger(E->getArg(0), Val, Info))
       return false;
     if (!Val)
       return Error(E);
 
     return Success(Val.countLeadingZeros(), E);
   }
 
-  case Builtin::BI__builtin_constant_p:
-    return Success(EvaluateBuiltinConstantP(Info.Ctx, E->getArg(0)), E);
+  case Builtin::BI__builtin_constant_p: {
+    auto Arg = E->getArg(0);
+    if (EvaluateBuiltinConstantP(Info.Ctx, Arg))
+      return Success(true, E);
+    auto ArgTy = Arg->IgnoreImplicit()->getType();
+    if (!Info.InConstantContext && !Arg->HasSideEffects(Info.Ctx) &&
+        !ArgTy->isAggregateType() && !ArgTy->isPointerType()) {
+      // We can delay calculation of __builtin_constant_p until after
+      // inlining. Note: This diagnostic won't be shown to the user.
+      Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
+      return false;
+    }
+    return Success(false, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
   case Builtin::BI__builtin_ctzll:
   case Builtin::BI__builtin_ctzs: {
     APSInt Val;
     if (!EvaluateInteger(E->getArg(0), Val, Info))
       return false;
if (Exp->isRValue() && (Exp->getType()->isArrayType() ||
                           Exp->getType()->isRecordType()) &&
       !Ctx.getLangOpts().CPlusPlus11) {
     IsConst = false;
     return true;
   }
   return false;
 }
 
+static bool EvaluateAsRValue(const Expr *E, Expr::EvalResult &Result,
+                             const ASTContext &Ctx, EvalInfo &Info) {
+  bool IsConst;
+  if (FastEvaluateAsRValue(E, Result, Ctx, IsConst))
+    return IsConst;
+
+  return EvaluateAsRValue(Info, E, Result.Val);
+}
 
 /// EvaluateAsRValue - Return true if this is a constant which we can fold using
 /// any crazy technique (that has nothing to do with language standards) that
 /// we want to.  If this function returns true, it returns the folded constant
 /// in Result. If this expression is a glvalue, an lvalue-to-rvalue conversion
 /// will be applied to the result.
 bool Expr::EvaluateAsRValue(EvalResult &Result, const ASTContext &Ctx) const {
-  bool IsConst;
-  if (FastEvaluateAsRValue(this, Result, Ctx, IsConst))
-    return IsConst;
-
   EvalInfo Info(Ctx, Result, EvalInfo::EM_IgnoreSideEffects);
-  return ::EvaluateAsRValue(Info, this, Result.Val);
+  return ::EvaluateAsRValue(this, Result, Ctx, Info);
 }
 
 bool Expr::EvaluateAsBooleanCondition(bool &Result,
                                       const ASTContext &Ctx) const {
   EvalResult Scratch;
   return EvaluateAsRValue(Scratch, Ctx) &&
          HandleConversionToBool(Scratch.Val, Result);
 }

 bool Expr::isEvaluatable(const ASTContext &Ctx, SideEffectsKind SEK) const {
   EvalResult Result;
   return EvaluateAsRValue(Result, Ctx) &&
          !hasUnacceptableSideEffect(Result, SEK);
 }
 
 APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx,
                     SmallVectorImpl<PartialDiagnosticAt> *Diag) const {
-  EvalResult EvalResult;
-  EvalResult.Diag = Diag;
-  bool Result = EvaluateAsRValue(EvalResult, Ctx);
+  EvalResult EVResult;
+  EVResult.Diag = Diag;
+  EvalInfo Info(Ctx, EVResult, EvalInfo::EM_IgnoreSideEffects);
+  Info.InConstantContext = true;
+
+  bool Result = ::EvaluateAsRValue(this, EVResult, Ctx, Info);
   (void)Result;
   assert(Result && "Could not evaluate expression");
-  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+  assert(EVResult.Val.isInt() && "Expression did not evaluate to integer");
 
-  return EvalResult.Val.getInt();
+  return EVResult.Val.getInt();
 }
 
 APSInt Expr::EvaluateKnownConstIntCheckOverflow(
     const ASTContext &Ctx, SmallVectorImpl<PartialDiagnosticAt> *Diag) const {
-  EvalResult EvalResult;
-  EvalResult.Diag = Diag;
-  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
-  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  EvalResult EVResult;
+  EVResult.Diag = Diag;
+  EvalInfo Info(Ctx, EVResult, EvalInfo::EM_EvaluateForOverflow);
+  Info.InConstantContext = true;
+
+  bool Result = ::EvaluateAsRValue(Info, this, EVResult.Val);
   (void)Result;
   assert(Result && "Could not evaluate expression");
-  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+  assert(EVResult.Val.isInt() && "Expression did not evaluate to integer");
 
-  return EvalResult.Val.getInt();
+  return EVResult.Val.getInt();
 }
 
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
-  EvalResult EvalResult;
-  if (!FastEvaluateAsRValue(this, EvalResult, Ctx, IsConst)) {
-    EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
-    (void)::EvaluateAsRValue(Info, this, EvalResult.Val);
+  EvalResult EVResult;
+  if (!FastEvaluateAsRValue(this, EVResult, Ctx, IsConst)) {
+    EvalInfo Info(Ctx, EVResult, EvalInfo::EM_EvaluateForOverflow);
+    (void)::EvaluateAsRValue(Info, this, EVResult.Val);
   }
 }
 
 bool Expr::EvalResult::isGlobalLValue() const {
   assert(Val.isLValue());
   return IsGlobalLValue(Val.getLValueBase());
 }
 

 }
 
 static ICEDiag NoDiag() { return ICEDiag(IK_ICE, SourceLocation()); }
 
 static ICEDiag Worst(ICEDiag A, ICEDiag B) { return A.Kind >= B.Kind ? A : B; }
 
 static ICEDiag CheckEvalInICE(const Expr* E, const ASTContext &Ctx) {
   Expr::EvalResult EVResult;
-  if (!E->EvaluateAsRValue(EVResult, Ctx) || EVResult.HasSideEffects ||
+  Expr::EvalStatus Status;
+  EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpression);
+
+  Info.InConstantContext = true;
+  if (!::EvaluateAsRValue(E, EVResult, Ctx, Info) || EVResult.HasSideEffects ||
       !EVResult.Val.isInt())
     return ICEDiag(IK_NotICE, E->getBeginLoc());
 
   return NoDiag();
 }
 
 static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
   assert(!E->isValueDependent() && "Should not see value dependent exprs!");
case Expr::CXXDefaultArgExprClass:
     return CheckICE(cast<CXXDefaultArgExpr>(E)->getExpr(), Ctx);
   case Expr::CXXDefaultInitExprClass:
     return CheckICE(cast<CXXDefaultInitExpr>(E)->getExpr(), Ctx);
   case Expr::ChooseExprClass: {
     return CheckICE(cast<ChooseExpr>(E)->getChosenSubExpr(), Ctx);
   }
   }
 
   llvm_unreachable("Invalid StmtClass!");
 }
 
 /// Evaluate an expression as a C++11 integral constant expression.
 static bool EvaluateCPlusPlus11IntegralConstantExpr(const ASTContext &Ctx,
                                                     const Expr *E,
                                                     llvm::APSInt *Value,
                                                     SourceLocation *Loc) {
   if (!E->getType()->isIntegralOrUnscopedEnumerationType()) {

lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_rotateleft64:
     return emitRotate(E, false);
 
   case Builtin::BI__builtin_rotateright8:
   case Builtin::BI__builtin_rotateright16:
   case Builtin::BI__builtin_rotateright32:
   case Builtin::BI__builtin_rotateright64:
     return emitRotate(E, true);
 
+  case Builtin::BI__builtin_constant_p: {
+    llvm::Type *ResultType = ConvertType(E->getType());
+    if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+      // At -O0, we don't perform inlining, so we don't need to delay the
+      // processing.
+      return RValue::get(ConstantInt::get(ResultType, 0));
+    if (auto *DRE = dyn_cast<DeclRefExpr>(E->getArg(0)->IgnoreImplicit())) {
+      auto DREType = DRE->getType();
+      if (DREType->isAggregateType() || DREType->isFunctionType())
+        return RValue::get(ConstantInt::get(ResultType, 0));
+    }
+    Value *ArgValue = EmitScalarExpr(E->getArg(0));
+    llvm::Type *ArgType = ArgValue->getType();
+
+    Value *F = CGM.getIntrinsic(Intrinsic::is_constant, ArgType);
+    Value *Result = Builder.CreateCall(F, ArgValue);
+    if (Result->getType() != ResultType)
+      Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true);
+    return RValue::get(Result);
+  }
   case Builtin::BI__builtin_object_size: {
     unsigned Type =
         E->getArg(1)->EvaluateKnownConstInt(getContext()).getZExtValue();
     auto *ResType = cast<llvm::IntegerType>(ConvertType(E->getType()));
 
     // We pass this builtin onto the optimizer so that it can figure out the
     // object size in more complex cases.
     return RValue::get(emitBuiltinObjectSize(E->getArg(0), Type, ResType,

lib/Sema/SemaDeclCXX.cpp

Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
   assert(AssertExpr != nullptr && "Expected non-null condition");
   if (!AssertExpr->isTypeDependent() && !AssertExpr->isValueDependent() &&
       !Failed) {
     // In a static_assert-declaration, the constant-expression shall be a
     // constant expression that can be contextually converted to bool.
     ExprResult Converted = PerformContextuallyConvertToBool(AssertExpr);
     if (Converted.isInvalid())
       Failed = true;
+    else
+      Converted = ConstantExpr::Create(Context, Converted.get());
 
     llvm::APSInt Cond;
     if (!Failed && VerifyIntegerConstantExpression(Converted.get(), &Cond,
           diag::err_static_assert_expression_is_not_constant,
           /*AllowFold=*/false).isInvalid())
       Failed = true;
 
     if (!Failed && !Cond) {

lib/Sema/SemaExpr.cpp

for (unsigned i = FirstParam; i < NumParams; i++) {
     // Check for array bounds violations for each argument to the call. This
     // check only triggers warnings when the argument isn't a more complex Expr
     // with its own checking, such as a BinaryOperator.
     CheckArrayAccess(Arg);
 
     // Check for violations of C99 static array rules (C99 6.7.5.3p7).
     CheckStaticArrayArgument(CallLoc, Param, Arg);
 
     AllArgs.push_back(Arg);
   }
 
   // If this is a variadic call, handle args passed through "...".
   if (CallType != VariadicDoesNotApply) {
     // Assume that extern "C" functions with variadic arguments that
     // return __unknown_anytype aren't *really* variadic.
     if (Proto->getReturnType() == Context.UnknownAnyTy && FDecl &&
         FDecl->isExternC()) {
       for (Expr *A : Args.slice(ArgIx)) {
Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo,
   // literal.
   // FIXME: GCC supports compound literals of reference type, which should
   // obviously have a value kind derived from the kind of reference involved.
   ExprValueKind VK =
       (getLangOpts().CPlusPlus && !(isFileScope && literalType->isArrayType()))
           ? VK_RValue
           : VK_LValue;
 
+  if (isFileScope)
+    LiteralExpr = ConstantExpr::Create(Context, LiteralExpr);
   Expr *E = new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literalType,
                                               VK, LiteralExpr, isFileScope);
   if (isFileScope) {
     if (!LiteralExpr->isTypeDependent() &&
         !LiteralExpr->isValueDependent() &&
         !literalType->isDependentType()) // C99 6.5.2.5p3
       if (CheckForConstantInitializer(LiteralExpr, literalType))
         return ExprError();
-    E = new (Context) ConstantExpr(E);
   } else if (literalType.getAddressSpace() != LangAS::opencl_private &&
              literalType.getAddressSpace() != LangAS::Default) {
     // Embedded-C extensions to C99 6.5.2.5:
     //   "If the compound literal occurs inside the body of a function, the
     //   type name shall not be qualified by an address-space qualifier."
     Diag(LParenLoc, diag::err_compound_literal_with_address_space)
       << SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd());
     return ExprError();
if (!E->getType()->isIntegralOrUnscopedEnumerationType())
       return ExprError();
   } else if (!E->getType()->isIntegralOrUnscopedEnumerationType()) {
     // An ICE must be of integral or unscoped enumeration type.
     if (!Diagnoser.Suppress)
       Diagnoser.diagnoseNotICE(*this, DiagLoc, E->getSourceRange());
     return ExprError();
   }
 
+  if (!isa<ConstantExpr>(E))
+    E = ConstantExpr::Create(Context, E);
+
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
     if (Result)
       *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
-    return new (Context) ConstantExpr(E);
+    return E;
   }
 
   Expr::EvalResult EvalResult;
   SmallVector<PartialDiagnosticAt, 8> Notes;
   EvalResult.Diag = &Notes;
 
   // Try to evaluate the expression, and produce diagnostics explaining why it's
   // not a constant expression as a side-effect.
   bool Folded = E->EvaluateAsRValue(EvalResult, Context) &&
                 EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
 
   // In C++11, we can rely on diagnostics being produced for any expression
   // which is not a constant expression. If no diagnostics were produced, then
   // this is a constant expression.
   if (Folded && getLangOpts().CPlusPlus11 && Notes.empty()) {
     if (Result)
       *Result = EvalResult.Val.getInt();
-    return new (Context) ConstantExpr(E);
+    return E;
   }
 
   // If our only note is the usual "invalid subexpression" note, just point
   // the caret at its location rather than producing an essentially
   // redundant note.
   if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
         diag::note_invalid_subexpr_in_const_expr) {
     DiagLoc = Notes[0].first;
Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
   }
 
   Diagnoser.diagnoseFold(*this, DiagLoc, E->getSourceRange());
   for (const PartialDiagnosticAt &Note : Notes)
     Diag(Note.first, Note.second);
 
   if (Result)
     *Result = EvalResult.Val.getInt();
-  return new (Context) ConstantExpr(E);
+  return E;
 }
 
 namespace {
   // Handle the case where we conclude a expression which we speculatively
   // considered to be unevaluated is actually evaluated.
   class TransformToPE : public TreeTransform<TransformToPE> {
     typedef TreeTransform<TransformToPE> BaseTransform;
 

lib/Sema/SemaOverload.cpp

if (!Result.get()->EvaluateAsConstantExpr(Eval, Usage, S.Context) ||
     // The expression can't be folded, so we can't keep it at this position in
     // the AST.
     Result = ExprError();
   } else {
     Value = Eval.Val;
 
     if (Notes.empty()) {
       // It's a constant expression.
-      return new (S.Context) ConstantExpr(Result.get());
+      return ConstantExpr::Create(S.Context, Result.get());
     }
   }
 
   // It's not a constant expression. Produce an appropriate diagnostic.
   if (Notes.size() == 1 &&
       Notes[0].second.getDiagID() == diag::note_invalid_subexpr_in_const_expr)
     S.Diag(Notes[0].first, diag::err_expr_not_cce) << CCE;
   else {

lib/Sema/SemaTemplateDeduction.cpp

 /// non-type template parameter.
 static NonTypeTemplateParmDecl *
 getDeducedParameterFromExpr(TemplateDeductionInfo &Info, Expr *E) {
   // If we are within an alias template, the expression may have undergone
   // any number of parameter substitutions already.
   while (true) {
     if (ImplicitCastExpr *IC = dyn_cast<ImplicitCastExpr>(E))
       E = IC->getSubExpr();
+    else if (ConstantExpr *CE = dyn_cast<ConstantExpr>(E))
+      E = CE->getSubExpr();
     else if (SubstNonTypeTemplateParmExpr *Subst =
                dyn_cast<SubstNonTypeTemplateParmExpr>(E))
       E = Subst->getReplacement();
     else
       break;
   }
 
   if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
MarkUsedTemplateParameters(ASTContext &Ctx,
   if (const PackExpansionExpr *Expansion = dyn_cast<PackExpansionExpr>(E))
     E = Expansion->getPattern();
 
   // Skip through any implicit casts we added while type-checking, and any
   // substitutions performed by template alias expansion.
   while (true) {
     if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
       E = ICE->getSubExpr();
+    else if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(E))
+      E = CE->getSubExpr();
     else if (const SubstNonTypeTemplateParmExpr *Subst =
                dyn_cast<SubstNonTypeTemplateParmExpr>(E))
       E = Subst->getReplacement();
     else
       break;
   }
 
   // FIXME: if !OnlyDeduced, we have to walk the whole subexpression to

lib/Sema/SemaType.cpp

if (ConstVal == 0) {
             << ConstVal.toString(10) << ArraySize->getSourceRange();
         return QualType();
       }
     }
 
     T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-    // A file-scoped array must have a constant array size.
-    ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
     Diag(Loc, diag::err_opencl_vla);
     return QualType();
   }
 
   if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) {
     if (getLangOpts().CUDA) {

lib/StaticAnalyzer/Core/ExprEngine.cpp

switch (S->getStmtClass()) {
     }
 
     case Stmt::ObjCAtSynchronizedStmtClass:
       Bldr.takeNodes(Pred);
       VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S), Pred, Dst);
       Bldr.addNodes(Dst);
       break;
 
     case Expr::ConstantExprClass:
-      // Handled due to it being a wrapper class.
-      break;
-
     case Stmt::ExprWithCleanupsClass:
       // Handled due to fully linearised CFG.
       break;
 
     case Stmt::CXXBindTemporaryExprClass: {
       Bldr.takeNodes(Pred);
       ExplodedNodeSet PreVisit;
       getCheckerManager().runCheckersForPreStmt(PreVisit, Pred, S, *this);
       ExplodedNodeSet Next;

test/Analysis/builtin-functions.cpp

void g(int i) {
   }
 }
 
 void test_constant_p() {
   int i = 1;
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }

test/Sema/builtins.c

 }
 
 // PR7885
 int test16() {
   return __builtin_constant_p() + // expected-error{{too few arguments}}
          __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
 typedef int __attribute__((vector_size(16))) IntVector;
 struct Aggregate { int n; char c; };
 enum Enum { EnumValue1, EnumValue2 };
 
 typedef __typeof(sizeof(int)) size_t;
#define F(...) ASSERT(!__builtin_constant_p(__VA_ARGS__))
   // ... and otherwise returns 0.
   F("string literal" + 1);
   F(&test17_n);
   F(test17_c);
   F(&test17_c);
   F(&test17_d);
   F((struct Aggregate){0, 1});
   F((IntVector){0, 1, 2, 3});
+  F(test17);
 
   // Ensure that a technique used in glibc is handled correctly.
 #define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
   // FIXME: These are incorrectly treated as ICEs because strlen is treated as
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
   // In these cases, the strlen is non-constant, but the __builtin_constant_p

test/SemaCXX/compound-literal.cpp

namespace brace_initializers {
   };
   struct HasCtorDtor {
     HasCtorDtor(int x, int y);
     ~HasCtorDtor();
   };
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
   void test() {
     (void)(POD){1, 2};
     // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
     // CHECK-NOT: ConstantExpr {{.*}} 'brace_initializers::POD'