Skip to content

Commit

Permalink
PR45534: don't ignore unmodeled side-effects when constant-evaluating…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
zygoloid committed Apr 21, 2020
1 parent b14e9e3 commit 4b03dd7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
2 changes: 1 addition & 1 deletion clang/lib/AST/ExprConstant.cpp
Expand Up @@ -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;
}
Expand Down
31 changes: 31 additions & 0 deletions 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__;

Expand Down Expand Up @@ -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

0 comments on commit 4b03dd7

Please sign in to comment.