[clang] [clang][ExprConst] Allow mutation in `__builtin_constant_p`'s argument (PR #159599)

[Timm Baeder via cfe-commits]

https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/159599

Allow side effects and mutation of local variables in the argument of a `__builtin_constant_p` call.

This aligns clang's behavior with GCC's current behavior: https://godbolt.org/z/8xhMxY6rx



>From 893e7137688f1cecac59da3eaec789ecd8a1056c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 18 Sep 2025 17:50:18 +0200
Subject: [PATCH] asdf

---
 clang/lib/AST/ExprConstant.cpp                |  7 +-----
 .../test/AST/ByteCode/builtin-constant-p.cpp  |  6 ++---
 clang/test/SemaCXX/builtin-constant-p.cpp     | 22 +++++--------------
 3 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index bea592d04c2df..ae53e921ffe8c 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4566,13 +4566,8 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
   //
   // FIXME: Not all local state is mutable. Allow local constant subobjects
   // to be read here (but take care with 'mutable' fields).
-  unsigned VisibleDepth = Depth;
-  if (llvm::isa_and_nonnull<ParmVarDecl>(
-          LVal.Base.dyn_cast<const ValueDecl *>()))
-    ++VisibleDepth;
   if ((Frame && Info.getLangOpts().CPlusPlus14 &&
-       Info.EvalStatus.HasSideEffects) ||
-      (isModification(AK) && VisibleDepth < Info.SpeculativeEvaluationDepth))
+       Info.EvalStatus.HasSideEffects))
     return CompleteObject();
 
   return CompleteObject(LVal.getLValueBase(), BaseVal, BaseType);
diff --git a/clang/test/AST/ByteCode/builtin-constant-p.cpp b/clang/test/AST/ByteCode/builtin-constant-p.cpp
index 315a907949c34..3f0f203735640 100644
--- a/clang/test/AST/ByteCode/builtin-constant-p.cpp
+++ b/clang/test/AST/ByteCode/builtin-constant-p.cpp
@@ -103,8 +103,7 @@ constexpr int mutate1() {
   int m = __builtin_constant_p(++n);
   return n * 10 + m;
 }
-static_assert(mutate1() == 21); // ref-error {{static assertion failed}} \
-                                // ref-note {{evaluates to '10 == 21'}}
+static_assert(mutate1() == 21);
 
 /// Similar for this. GCC agrees with the bytecode interpreter.
 constexpr int mutate_param(bool mutate, int &param) {
@@ -119,8 +118,7 @@ constexpr int mutate6(bool mutate) {
   return n * 10 + m;
 }
 static_assert(mutate6(false) == 11);
-static_assert(mutate6(true) == 21); // ref-error {{static assertion failed}} \
-                                    // ref-note {{evaluates to '10 == 21'}}
+static_assert(mutate6(true) == 21);
 
 #define fold(x) (__builtin_constant_p(x) ? (x) : (x))
 void g() {
diff --git a/clang/test/SemaCXX/builtin-constant-p.cpp b/clang/test/SemaCXX/builtin-constant-p.cpp
index 71c38c5c63c71..af56a805bd0d4 100644
--- a/clang/test/SemaCXX/builtin-constant-p.cpp
+++ b/clang/test/SemaCXX/builtin-constant-p.cpp
@@ -61,25 +61,14 @@ static_assert(bcp(int_to_ptr(123)));      // GCC rejects these due to not recogn
 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
 
-// State mutations in the operand are not permitted.
-//
-// The rule GCC uses for this is not entirely understood, but seems to depend
-// in some way on what local state is mentioned in the operand of
-// __builtin_constant_p and where.
-//
-// We approximate GCC's rule by evaluating the operand in a speculative
-// evaluation context; only state created within the evaluation can be
-// modified.
+// State mutations in the operand are permitted.
 constexpr int mutate1() {
   int n = 1;
   int m = __builtin_constant_p(++n);
   return n * 10 + m;
 }
-static_assert(mutate1() == 10);
+static_assert(mutate1() == 21);
 
-// FIXME: GCC treats this as being non-constant because of the "n = 2", even
-// though evaluation in the context of the enclosing constant expression
-// succeeds without mutating any state.
 constexpr int mutate2() {
   int n = 1;
   int m = __builtin_constant_p(n ? n + 1 : n = 2);
@@ -107,8 +96,6 @@ constexpr int mutate4() {
 }
 static_assert(mutate4() == 11);
 
-// FIXME: GCC treats this as being non-constant because of something to do with
-// the 'n' in the argument to internal_mutation.
 constexpr int mutate5() {
   int n = 1;
   int m = __builtin_constant_p(n ? internal_mutation(n) : 0);
@@ -130,7 +117,7 @@ constexpr int mutate6(bool mutate) {
 // No mutation of state outside __builtin_constant_p: evaluates to true.
 static_assert(mutate6(false) == 11);
 // Mutation of state outside __builtin_constant_p: evaluates to false.
-static_assert(mutate6(true) == 10);
+static_assert(mutate6(true) == 21);
 
 // GCC strangely returns true for the address of a type_info object, despite it
 // not being a pointer to the start of a string literal.
@@ -151,6 +138,9 @@ namespace dtor_side_effect {
 }
 
 #if __cplusplus >= 202002L
+
+static_assert(__builtin_constant_p((delete new int, 0))); // expected-error {{failed}}
+
 namespace constexpr_dtor {
   struct A {
     int *p;