PR48587: is_constant_evaluated() should not evaluate to true during a

variable's destruction if it didn't do so during construction.

The standard doesn't give any guidance as to what to do here, but this
approach seems reasonable and conservative, and has been proposed to the
standard committee.

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b19f112..1ed9bbe 100644 (file)
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14799,11 +14799,14 @@ bool Expr::EvaluateAsLValue(EvalResult &Result, const ASTContext &Ctx,
 
 static bool EvaluateDestruction(const ASTContext &Ctx, APValue::LValueBase Base,
                                 APValue DestroyedValue, QualType Type,
-                                SourceLocation Loc, Expr::EvalStatus &EStatus) {
-  EvalInfo Info(Ctx, EStatus, EvalInfo::EM_ConstantExpression);
+                                SourceLocation Loc, Expr::EvalStatus &EStatus,
+                                bool IsConstantDestruction) {
+  EvalInfo Info(Ctx, EStatus,
+                IsConstantDestruction ? EvalInfo::EM_ConstantExpression
+                                      : EvalInfo::EM_ConstantFold);
   Info.setEvaluatingDecl(Base, DestroyedValue,
                          EvalInfo::EvaluatingDeclKind::Dtor);
-  Info.InConstantContext = true;
+  Info.InConstantContext = IsConstantDestruction;
 
   LValue LVal;
   LVal.set(Base);
@@ -14857,7 +14860,8 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result, const ASTContext &Ctx,
   // If this is a class template argument, it's required to have constant
   // destruction too.
   if (Kind == ConstantExprKind::ClassTemplateArgument &&
-      (!EvaluateDestruction(Ctx, Base, Result.Val, T, getBeginLoc(), Result) ||
+      (!EvaluateDestruction(Ctx, Base, Result.Val, T, getBeginLoc(), Result,
+                            true) ||
        Result.HasSideEffects)) {
     // FIXME: Prefix a note to indicate that the problem is lack of constant
     // destruction.
@@ -14923,6 +14927,10 @@ bool VarDecl::evaluateDestruction(
   Expr::EvalStatus EStatus;
   EStatus.Diag = &Notes;
 
+  // Only treat the destruction as constant destruction if we formally have
+  // constant initialization (or are usable in a constant expression).
+  bool IsConstantDestruction = hasConstantInitialization();
+
   // Make a copy of the value for the destructor to mutate, if we know it.
   // Otherwise, treat the value as default-initialized; if the destructor works
   // anyway, then the destruction is constant (and must be essentially empty).
@@ -14933,7 +14941,8 @@ bool VarDecl::evaluateDestruction(
     return false;
 
   if (!EvaluateDestruction(getASTContext(), this, std::move(DestroyedValue),
-                           getType(), getLocation(), EStatus) ||
+                           getType(), getLocation(), EStatus,
+                           IsConstantDestruction) ||
       EStatus.HasSideEffects)
     return false;
 

diff --git a/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp b/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
index 967c834..d30fefe 100644 (file)
--- a/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
+++ b/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
@@ -4,6 +4,7 @@
 // RUN: FileCheck -check-prefix=CHECK-DYN -input-file=%t.ll %s
 // RUN: FileCheck -check-prefix=CHECK-ARR -input-file=%t.ll %s
 // RUN: FileCheck -check-prefix=CHECK-FOLD -input-file=%t.ll %s
+// RUN: FileCheck -check-prefix=CHECK-DTOR -input-file=%t.ll %s
 
 using size_t = decltype(sizeof(int));
 
@@ -131,3 +132,94 @@ void test_ref_to_static_var() {
   // CHECK-FOLD: store i32* @_ZZ22test_ref_to_static_varvE10i_constant, i32** %r,
   int &r = __builtin_is_constant_evaluated() ? i_constant : i_non_constant;
 }
+
+int not_constexpr;
+
+// __builtin_is_constant_evaluated() should never evaluate to true during
+// destruction if it would not have done so during construction.
+//
+// FIXME: The standard doesn't say that it should ever return true when
+// evaluating a destructor call, even for a constexpr variable. That seems
+// obviously wrong.
+struct DestructorBCE {
+  int n;
+  constexpr DestructorBCE(int n) : n(n) {}
+  constexpr ~DestructorBCE() {
+    if (!__builtin_is_constant_evaluated())
+      not_constexpr = 1;
+  }
+};
+
+// CHECK-DTOR-NOT: @_ZN13DestructorBCED{{.*}}@global_dtor_bce_1
+DestructorBCE global_dtor_bce_1(101);
+
+// CHECK-DTOR: load i32, i32* @not_constexpr
+// CHECK-DTOR: call {{.*}} @_ZN13DestructorBCEC1Ei({{.*}} @global_dtor_bce_2, i32
+// CHECK-DTOR: atexit{{.*}} @_ZN13DestructorBCED{{.*}} @global_dtor_bce_2
+// CHECK-DTOR: }
+DestructorBCE global_dtor_bce_2(not_constexpr);
+
+// CHECK-DTOR-NOT: @_ZN13DestructorBCED{{.*}}@global_dtor_bce_3
+constexpr DestructorBCE global_dtor_bce_3(103);
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z15test_dtor_bce_1v(
+void test_dtor_bce_1() {
+  // Variable is neither constant initialized (because it has automatic storage
+  // duration) nor usable in constant expressions, so BCE should not return
+  // true during destruction. It would be OK if we replaced the constructor
+  // call with a direct store, but we should emit the destructor call.
+
+  // CHECK-DTOR: call {{.*}} @_ZN13DestructorBCEC1Ei({{.*}}, i32 201)
+  DestructorBCE local(201);
+  // CHECK-DTOR: call {{.*}} @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z15test_dtor_bce_2v(
+void test_dtor_bce_2() {
+  // Non-constant init => BCE is false in destructor.
+
+  // CHECK-DTOR: call {{.*}} @_ZN13DestructorBCEC1Ei({{.*}}
+  DestructorBCE local(not_constexpr);
+  // CHECK-DTOR: call {{.*}} @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z15test_dtor_bce_3v(
+void test_dtor_bce_3() {
+  // Should never call dtor for a constexpr variable.
+
+  // CHECK-DTOR-NOT: call {{.*}} @_ZN13DestructorBCEC1Ei(
+  constexpr DestructorBCE local(203);
+  // CHECK-DTOR-NOT: @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z22test_dtor_bce_static_1v(
+void test_dtor_bce_static_1() {
+  // Variable is constant initialized, so BCE returns true during constant
+  // destruction.
+
+  // CHECK: store i32 301
+  // CHECK-DTOR-NOT: @_ZN13DestructorBCEC1Ei({{.*}}
+  static DestructorBCE local(301);
+  // CHECK-DTOR-NOT: @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z22test_dtor_bce_static_2v(
+void test_dtor_bce_static_2() {
+  // CHECK-DTOR: call {{.*}} @_ZN13DestructorBCEC1Ei({{.*}}
+  static DestructorBCE local(not_constexpr);
+  // CHECK-DTOR: call {{.*}}atexit{{.*}} @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}
+
+// CHECK-DTOR-LABEL: define {{.*}} @_Z22test_dtor_bce_static_3v(
+void test_dtor_bce_static_3() {
+  // CHECK: store i32 303
+  // CHECK-DTOR-NOT: @_ZN13DestructorBCEC1Ei({{.*}}
+  static constexpr DestructorBCE local(303);
+  // CHECK-DTOR-NOT: @_ZN13DestructorBCED
+  // CHECK-DTOR: }
+}