[clang] 4b03dd7 - PR45534: don't ignore unmodeled side-effects when constant-evaluating a call to __builtin_constant_p.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-04-20T21:23:35-07:00
New Revision: 4b03dd7b849e8f5068dc8d72c6eab724c22a2805

URL: https://github.com/llvm/llvm-project/commit/4b03dd7b849e8f5068dc8d72c6eab724c22a2805
DIFF: https://github.com/llvm/llvm-project/commit/4b03dd7b849e8f5068dc8d72c6eab724c22a2805.diff

LOG: PR45534: don't ignore unmodeled side-effects when constant-evaluating a call to __builtin_constant_p.

Such side-effects should result in the call evaluating to 'false', even
if we can still determine what value the argument expression will
evaluate to.

Added: 
    

Modified: 
    clang/lib/AST/ExprConstant.cpp
    clang/test/SemaCXX/builtin-constant-p.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8bc7a1128e7a..ad61221a6a91 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10438,7 +10438,7 @@ static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {
       ArgType->isAnyComplexType() || ArgType->isPointerType() ||
       ArgType->isNullPtrType()) {
     APValue V;
-    if (!::EvaluateAsRValue(Info, Arg, V)) {
+    if (!::EvaluateAsRValue(Info, Arg, V) || Info.EvalStatus.HasSideEffects) {
       Fold.keepDiagnostics();
       return false;
     }

diff  --git a/clang/test/SemaCXX/builtin-constant-p.cpp b/clang/test/SemaCXX/builtin-constant-p.cpp
index f70676d250e0..1b8cf455ef27 100644
--- a/clang/test/SemaCXX/builtin-constant-p.cpp
+++ b/clang/test/SemaCXX/builtin-constant-p.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s
 
 using intptr_t = __INTPTR_TYPE__;
 
@@ -135,3 +136,33 @@ static_assert(mutate6(true) == 10);
 // not being a pointer to the start of a string literal.
 namespace std { struct type_info; }
 static_assert(__builtin_constant_p(&typeid(int)));
+
+void mutate_as_side_effect() {
+  int a;
+  static_assert(!__builtin_constant_p(((void)++a, 1)));
+}
+
+namespace dtor_side_effect {
+  struct A {
+    constexpr A() {}
+    ~A();
+  };
+  static_assert(!__builtin_constant_p((A{}, 123)));
+}
+
+#if __cplusplus >= 202002L
+namespace constexpr_dtor {
+  struct A {
+    int *p;
+    constexpr ~A() { *p = 0; }
+  };
+  struct Q { int n; constexpr int *get() { return &n; } };
+  static_assert(!__builtin_constant_p((A{}, 123)));
+  // FIXME: We should probably accept this. GCC does.
+  // However, GCC appears to do so by running the destructors at the end of the
+  // enclosing full-expression, which seems broken; running them at the end of
+  // the evaluation of the __builtin_constant_p argument would be more
+  // defensible.
+  static_assert(!__builtin_constant_p((A{Q().get()}, 123)));
+}
+#endif