Reinstate r359059, reverted in r359361, with a fix to properly prevent

us emitting the operand of __builtin_constant_p if it has side-effects.

Original commit message:

Fix interactions between __builtin_constant_p and constexpr to match
current trunk GCC.

GCC permits information from outside the operand of
__builtin_constant_p (but in the same constant evaluation context) to be
used within that operand; clang now does so too. A few other minor
deviations from GCC's behavior showed up in my testing and are also
fixed (matching GCC):
  * Clang now supports nullptr_t as the argument type for
    __builtin_constant_p
    * Clang now returns true from __builtin_constant_p if called with a
    null pointer
    * Clang now returns true from __builtin_constant_p if called with an
    integer cast to pointer type

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

lib/AST/ExprConstant.cpp

@@ -7801,19 +7801,33 @@ EvaluateBuiltinClassifyType(const CallExpr *E, const LangOptions &LangOpts) {
 }
 
 /// EvaluateBuiltinConstantPForLValue - Determine the result of
-/// __builtin_constant_p when applied to the given lvalue.
+/// __builtin_constant_p when applied to the given pointer.
 ///
-/// An lvalue is only "constant" if it is a pointer or reference to the first
-/// character of a string literal.
-template<typename LValue>
-static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) {
-  const Expr *E = LV.getLValueBase().template dyn_cast<const Expr*>();
-  return E && isa<StringLiteral>(E) && LV.getLValueOffset().isZero();
+/// A pointer is only "constant" if it is null (or a pointer cast to integer)
+/// or it points to the first character of a string literal.
+static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) {
+  APValue::LValueBase Base = LV.getLValueBase();
+  if (Base.isNull()) {
+    // A null base is acceptable.
+    return true;
+  } else if (const Expr *E = Base.dyn_cast<const Expr *>()) {
+    if (!isa<StringLiteral>(E))
+      return false;
+    return LV.getLValueOffset().isZero();
+  } else {
+    // Any other base is not constant enough for GCC.
+    return false;
+  }
 }
 
 /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly to
 /// GCC as we can manage.
-static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
+static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {
+  // Constant-folding is always enabled for the operand of __builtin_constant_p
+  // (even when the enclosing evaluation context otherwise requires a strict
+  // language-specific constant expression).
+  FoldConstant Fold(Info, true);
+
   QualType ArgType = Arg->getType();
 
   // __builtin_constant_p always has one operand. The rules which gcc follows
@@ -7821,34 +7835,30 @@ static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
   //
   //  - If the operand is of integral, floating, complex or enumeration type,
   //    and can be folded to a known value of that type, it returns 1.
-  //  - If the operand and can be folded to a pointer to the first character
-  //    of a string literal (or such a pointer cast to an integral type), it
-  //    returns 1.
+  //  - If the operand can be folded to a pointer to the first character
+  //    of a string literal (or such a pointer cast to an integral type)
+  //    or to a null pointer or an integer cast to a pointer, it returns 1.
   //
   // Otherwise, it returns 0.
   //
   // FIXME: GCC also intends to return 1 for literals of aggregate types, but
-  // its support for this does not currently work.
-  if (ArgType->isIntegralOrEnumerationType()) {
-    Expr::EvalResult Result;
-    if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
+  // its support for this did not work prior to GCC 9 and is not yet well
+  // understood.
+  if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType() ||
+      ArgType->isAnyComplexType() || ArgType->isPointerType() ||
+      ArgType->isNullPtrType()) {
+    APValue V;
+    if (!::EvaluateAsRValue(Info, Arg, V)) {
+      Fold.keepDiagnostics();
       return false;
+    }
 
-    APValue &V = Result.Val;
-    if (V.getKind() == APValue::Int)
-      return true;
+    // For a pointer (possibly cast to integer), there are special rules.
     if (V.getKind() == APValue::LValue)
       return EvaluateBuiltinConstantPForLValue(V);
-  } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
-    return Arg->isEvaluatable(Ctx);
-  } else if (ArgType->isPointerType() || Arg->isGLValue()) {
-    LValue LV;
-    Expr::EvalStatus Status;
-    EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);
-    if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, Info)
-                          : EvaluatePointer(Arg, LV, Info)) &&
-        !Status.HasSideEffects)
-      return EvaluateBuiltinConstantPForLValue(LV);
+
+    // Otherwise, any constant value is good enough.
+    return V.getKind() != APValue::Uninitialized;
   }
 
   // Anything else isn't considered to be sufficiently constant.
@@ -8258,18 +8268,15 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
   }
 
   case Builtin::BI__builtin_constant_p: {
-    auto Arg = E->getArg(0);
-    if (EvaluateBuiltinConstantP(Info.Ctx, Arg))
+    const Expr *Arg = E->getArg(0);
+    if (EvaluateBuiltinConstantP(Info, 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.
+    else if (Info.InConstantContext)
+      return Success(false, E);
+    else {
       Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
       return false;
     }
-    return Success(false, E);
   }
 
   case Builtin::BI__builtin_is_constant_evaluated:

lib/CodeGen/CGBuiltin.cpp

@@ -2027,8 +2027,17 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
 
     const Expr *Arg = E->getArg(0);
     QualType ArgType = Arg->getType();
-    if (!hasScalarEvaluationKind(ArgType) || ArgType->isFunctionType())
-      // We can only reason about scalar types.
+    // FIXME: The allowance for Obj-C pointers and block pointers is historical
+    // and likely a mistake.
+    if (!ArgType->isIntegralOrEnumerationType() && !ArgType->isFloatingType() &&
+        !ArgType->isObjCObjectPointerType() && !ArgType->isBlockPointerType())
+      // Per the GCC documentation, only numeric constants are recognized after
+      // inlining.
+      return RValue::get(ConstantInt::get(ResultType, 0));
+
+    if (Arg->HasSideEffects(getContext()))
+      // The argument is unevaluated, so be conservative if it might have
+      // side-effects.
       return RValue::get(ConstantInt::get(ResultType, 0));
 
     Value *ArgValue = EmitScalarExpr(Arg);

lib/Sema/SemaChecking.cpp

@@ -1199,10 +1199,14 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
     if (checkArgCount(*this, TheCall, 1)) return true;
     TheCall->setType(Context.IntTy);
     break;
-  case Builtin::BI__builtin_constant_p:
+  case Builtin::BI__builtin_constant_p: {
     if (checkArgCount(*this, TheCall, 1)) return true;
+    ExprResult Arg = DefaultFunctionArrayLvalueConversion(TheCall->getArg(0));
+    if (Arg.isInvalid()) return true;
+    TheCall->setArg(0, Arg.get());
     TheCall->setType(Context.IntTy);
     break;
+  }
   case Builtin::BI__builtin_launder:
     return SemaBuiltinLaunder(*this, TheCall);
   case Builtin::BI__sync_fetch_and_add:

test/CodeGen/builtin-constant-p.c

@@ -177,3 +177,11 @@ void test17() {
   // CHECK: call void asm sideeffect "", {{.*}}(i32 -1) 
   __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
 }
+
+int test18_f();
+// CHECK: define void @test18
+// CHECK-NOT: call {{.*}}test18_f
+void test18() {
+  int a, b;
+  (void)__builtin_constant_p((a = b, test18_f()));
+}

test/SemaCXX/builtin-constant-p.cpp

@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -std=c++17 -verify %s
+
+using intptr_t = __INTPTR_TYPE__;
+
+// Test interaction of constexpr and __builtin_constant_p.
+
+template<typename T> constexpr bool bcp(T t) {
+  return __builtin_constant_p(t);
+}
+template<typename T> constexpr bool bcp_fold(T t) {
+  return __builtin_constant_p(((void)(intptr_t)&t, t));
+}
+
+constexpr intptr_t ensure_fold_is_generally_not_enabled = // expected-error {{constant expression}}
+    (intptr_t)&ensure_fold_is_generally_not_enabled; // expected-note {{cast}}
+
+constexpr intptr_t ptr_to_int(const void *p) {
+  return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p;
+}
+
+constexpr int *int_to_ptr(intptr_t n) {
+  return __builtin_constant_p(1) ? (int*)n : (int*)n;
+}
+
+int x;
+
+// Integer and floating point constants encountered during constant expression
+// evaluation are considered constant. So is nullptr_t.
+static_assert(bcp(1));
+static_assert(bcp_fold(1));
+static_assert(bcp(1.0));
+static_assert(bcp_fold(1.0));
+static_assert(bcp(nullptr));
+static_assert(bcp_fold(nullptr));
+
+// Pointers to the start of strings are considered constant.
+static_assert(bcp("foo"));
+static_assert(bcp_fold("foo"));
+
+// Null pointers are considered constant.
+static_assert(bcp<int*>(nullptr));
+static_assert(bcp_fold<int*>(nullptr));
+static_assert(bcp<const char*>(nullptr));
+static_assert(bcp_fold<const char*>(nullptr));
+
+// Other pointers are not.
+static_assert(!bcp(&x));
+static_assert(!bcp_fold(&x));
+
+// Pointers cast to integers follow the rules for pointers.
+static_assert(bcp(ptr_to_int("foo")));
+static_assert(bcp_fold(ptr_to_int("foo")));
+static_assert(!bcp(ptr_to_int(&x)));
+static_assert(!bcp_fold(ptr_to_int(&x)));
+
+// Integers cast to pointers follow the integer rules.
+static_assert(bcp(int_to_ptr(0)));
+static_assert(bcp_fold(int_to_ptr(0)));
+static_assert(bcp(int_to_ptr(123)));      // GCC rejects these due to not recognizing
+static_assert(bcp_fold(int_to_ptr(123))); // the bcp conditional in 'int_to_ptr' ...
+static_assert(__builtin_constant_p((int*)123)); // ... but GCC accepts this

test/SemaCXX/enable_if.cpp

@@ -522,3 +522,14 @@ void test() {
   InConstantContext::foo("abc");
 }
 } // namespace InConstantContext
+
+namespace StringLiteralDetector {
+  void need_string_literal(const char *p) __attribute__((enable_if(__builtin_constant_p(p), "argument is not a string literal"))); // expected-note 2{{not a string literal}}
+  void test(const char *unknown) {
+    need_string_literal("foo");
+    need_string_literal(unknown); // expected-error {{no matching function}}
+    constexpr char str[] = "bar";
+    need_string_literal(str); // expected-error {{no matching function}}
+  }
+}
+