[clang] 08c8d5b - Properly track whether a variable is constant-initialized.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 19 23:59:37 PDT 2020
Author: Richard Smith
Date: 2020-10-19T23:59:11-07:00
New Revision: 08c8d5bc51c512e605840b8003fcf38c86d0fc96
URL: https://github.com/llvm/llvm-project/commit/08c8d5bc51c512e605840b8003fcf38c86d0fc96
DIFF: https://github.com/llvm/llvm-project/commit/08c8d5bc51c512e605840b8003fcf38c86d0fc96.diff
LOG: Properly track whether a variable is constant-initialized.
This fixes miscomputation of __builtin_constant_evaluated in the
initializer of a variable that's not usable in constant expressions, but
is readable when constant-folding.
If evaluation of a constant initializer fails, we throw away the
evaluated result instead of keeping it as a non-constant-initializer
value for the variable, because it might not be a correct value.
To avoid regressions for initializers that are foldable but not formally
constant initializers, we now try constant-evaluating some globals in
C++ twice: once to check for a constant initializer (in an mode where
is_constannt_evaluated returns true) and again to determine the runtime
value if the initializer is not a constant initializer.
Added:
clang/test/PCH/builtin-is-constant-evaluated.cpp
Modified:
clang/include/clang/AST/Decl.h
clang/include/clang/AST/Expr.h
clang/lib/AST/Decl.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
clang/test/SemaCXX/attr-require-constant-initialization.cpp
clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index fe906fc8c6d69..0a9474c3ee86d 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -1273,12 +1273,15 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
EvaluatedStmt *getEvaluatedStmt() const;
/// Attempt to evaluate the value of the initializer attached to this
- /// declaration, and produce notes explaining why it cannot be evaluated or is
- /// not a constant expression. Returns a pointer to the value if evaluation
- /// succeeded, 0 otherwise.
+ /// declaration, and produce notes explaining why it cannot be evaluated.
+ /// Returns a pointer to the value if evaluation succeeded, 0 otherwise.
APValue *evaluateValue() const;
- APValue *evaluateValue(SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
+private:
+ APValue *evaluateValueImpl(SmallVectorImpl<PartialDiagnosticAt> &Notes,
+ bool IsConstantInitialization) const;
+
+public:
/// Return the already-evaluated value of this variable's
/// initializer, or NULL if the value is not yet known. Returns pointer
/// to untyped APValue if the value could not be evaluated.
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 4a8e4e483a862..25957909739ec 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -699,7 +699,8 @@ class Expr : public ValueStmt {
/// notes will be produced if the expression is not a constant expression.
bool EvaluateAsInitializer(APValue &Result, const ASTContext &Ctx,
const VarDecl *VD,
- SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
+ SmallVectorImpl<PartialDiagnosticAt> &Notes,
+ bool IsConstantInitializer) const;
/// EvaluateWithSubstitution - Evaluate an expression as if from the context
/// of a call to the given function with the given arguments, inside an
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5331ed8f0a9e1..5084f867622f4 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2360,11 +2360,11 @@ EvaluatedStmt *VarDecl::getEvaluatedStmt() const {
APValue *VarDecl::evaluateValue() const {
SmallVector<PartialDiagnosticAt, 8> Notes;
- return evaluateValue(Notes);
+ return evaluateValueImpl(Notes, hasConstantInitialization());
}
-APValue *VarDecl::evaluateValue(
- SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
+APValue *VarDecl::evaluateValueImpl(SmallVectorImpl<PartialDiagnosticAt> &Notes,
+ bool IsConstantInitialization) const {
EvaluatedStmt *Eval = ensureEvaluatedStmt();
const auto *Init = cast<Expr>(Eval->Value);
@@ -2383,8 +2383,16 @@ APValue *VarDecl::evaluateValue(
Eval->IsEvaluating = true;
- bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, getASTContext(),
- this, Notes);
+ ASTContext &Ctx = getASTContext();
+ bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, Ctx, this, Notes,
+ IsConstantInitialization);
+
+ // In C++11, this isn't a constant initializer if we produced notes. In that
+ // case, we can't keep the result, because it may only be correct under the
+ // assumption that the initializer is a constant context.
+ if (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus11 &&
+ !Notes.empty())
+ Result = false;
// Ensure the computed APValue is cleaned up later if evaluation succeeded,
// or that it's empty (so that there's nothing to clean up) if evaluation
@@ -2392,7 +2400,7 @@ APValue *VarDecl::evaluateValue(
if (!Result)
Eval->Evaluated = APValue();
else if (Eval->Evaluated.needsCleanup())
- getASTContext().addDestruction(&Eval->Evaluated);
+ Ctx.addDestruction(&Eval->Evaluated);
Eval->IsEvaluating = false;
Eval->WasEvaluated = true;
@@ -2447,7 +2455,14 @@ bool VarDecl::checkForConstantInitialization(
assert(!Init->isValueDependent());
// Evaluate the initializer to check whether it's a constant expression.
- Eval->HasConstantInitialization = evaluateValue(Notes) && Notes.empty();
+ Eval->HasConstantInitialization =
+ evaluateValueImpl(Notes, true) && Notes.empty();
+
+ // If evaluation as a constant initializer failed, allow re-evaluation as a
+ // non-constant initializer if we later find we want the value.
+ if (!Eval->HasConstantInitialization)
+ Eval->WasEvaluated = false;
+
return Eval->HasConstantInitialization;
}
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 34893302f2e79..f4664bb624148 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3269,12 +3269,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
// Check that we can fold the initializer. In C++, we will have already done
// this in the cases where it matters for conformance.
- SmallVector<PartialDiagnosticAt, 8> Notes;
- if (!VD->evaluateValue(Notes)) {
- Info.FFDiag(E, diag::note_constexpr_var_init_non_constant,
- Notes.size() + 1) << VD;
+ if (!VD->evaluateValue()) {
+ Info.FFDiag(E, diag::note_constexpr_var_init_non_constant, 1) << VD;
NoteLValueLocation(Info, Base);
- Info.addNotes(Notes);
return false;
}
@@ -14687,7 +14684,8 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result, ConstExprUsage Usage,
bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
const VarDecl *VD,
- SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
+ SmallVectorImpl<PartialDiagnosticAt> &Notes,
+ bool IsConstantInitialization) const {
assert(!isValueDependent() &&
"Expression evaluator can't be called on a dependent expression.");
@@ -14700,11 +14698,12 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
Expr::EvalStatus EStatus;
EStatus.Diag = &Notes;
- EvalInfo Info(Ctx, EStatus, VD->isConstexpr()
- ? EvalInfo::EM_ConstantExpression
- : EvalInfo::EM_ConstantFold);
+ EvalInfo Info(Ctx, EStatus,
+ (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus11)
+ ? EvalInfo::EM_ConstantExpression
+ : EvalInfo::EM_ConstantFold);
Info.setEvaluatingDecl(VD, Value);
- Info.InConstantContext = true;
+ Info.InConstantContext = IsConstantInitialization;
SourceLocation DeclLoc = VD->getLocation();
QualType DeclTy = VD->getType();
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 57f1ad59a72a4..011d39cfa2947 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1622,8 +1622,8 @@ llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
if (CD->isTrivial() && CD->isDefaultConstructor())
return CGM.EmitNullConstant(D.getType());
}
- InConstantContext = true;
}
+ InConstantContext = D.hasConstantInitialization();
QualType destType = D.getType();
diff --git a/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp b/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
index cc4e375b50ad4..78dfc695c15e4 100644
--- a/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
+++ b/clang/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
@@ -63,9 +63,9 @@ CONSTINIT int b = __builtin_is_constant_evaluated() ? 2 : y; // static initializ
// CHECK-DYN-NEXT: store i32 %add, i32* @c,
int c = y + (__builtin_is_constant_evaluated() ? 2 : y); // dynamic initialization to y+y
-// CHECK-DYN-LABEL: define internal void @__cxx_global_var_init.2()
-// CHECK-DYN: store i32 1, i32* @_ZL1a, align 4
-// CHECK-DYN-NEXT: ret void
+// This is dynamic initialization that we can convert to static initialization
+// during lowering. When doing so, the dynamic initializer value is preserved.
+// CHECK-STATIC-DAG: @_ZL1a = internal constant i32 1
const int a = __builtin_is_constant_evaluated() ? y : 1; // dynamic initialization to 1
const int *a_sink = &a;
diff --git a/clang/test/PCH/builtin-is-constant-evaluated.cpp b/clang/test/PCH/builtin-is-constant-evaluated.cpp
new file mode 100644
index 0000000000000..667d586c2578d
--- /dev/null
+++ b/clang/test/PCH/builtin-is-constant-evaluated.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++98 -Wno-constant-evaluated -triple x86_64-linux -include %s -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++98 -Wno-constant-evaluated -triple x86_64-linux -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++98 -Wno-constant-evaluated -triple x86_64-linux -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -std=c++11 -Wno-constant-evaluated -triple x86_64-linux -include %s -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CXX11
+// RUN: %clang_cc1 -std=c++11 -Wno-constant-evaluated -triple x86_64-linux -emit-pch %s -o %t-cxx11
+// RUN: %clang_cc1 -std=c++11 -Wno-constant-evaluated -triple x86_64-linux -include-pch %t-cxx11 -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CXX11
+
+// RUN: %clang_cc1 -std=c++20 -Wno-constant-evaluated -triple x86_64-linux -include %s -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CXX11
+// RUN: %clang_cc1 -std=c++20 -Wno-constant-evaluated -triple x86_64-linux -emit-pch %s -o %t-cxx11
+// RUN: %clang_cc1 -std=c++20 -Wno-constant-evaluated -triple x86_64-linux -include-pch %t-cxx11 -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,CXX11
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+// CHECK-DAG: @a = global i8 1,
+// CHECK-DAG: @b = constant i8 1,
+// CXX11-DAG: @c = constant i8 1,
+// CHECK-DAG: @d = global float 1.000000e+00
+// CHECK-DAG: @e = constant float 1.000000e+00
+
+bool a = __builtin_is_constant_evaluated();
+extern const bool b = __builtin_is_constant_evaluated();
+#if __cplusplus >= 201103L
+extern constexpr bool c = __builtin_is_constant_evaluated();
+#endif
+float d = __builtin_is_constant_evaluated();
+extern const float e = __builtin_is_constant_evaluated();
+
+void g(...);
+
+// CHECK-LABEL: define {{.*}} @_Z1fv(
+// CHECK: store i8 0, i8* %[[A:.*]],
+// CHECK: store i8 1, i8* %[[B:.*]],
+// CXX11: store i8 1, i8* %[[C:.*]],
+// CHECK: store float 0.000000e+00, float* %[[D:.*]],
+// CHECK: store float 0.000000e+00, float* %[[E:.*]],
+// CHECK: load i8, i8* %[[A]],
+// CHECK: call {{.*}} @_Z1gz(i32 %{{[^,]+}}, i32 1
+// CXX11-SAME: , i32 1
+// CHECK-SAME: , double %{{[^,]+}}, double 0.000000e+00)
+void f() {
+ bool a = __builtin_is_constant_evaluated();
+ const bool b = __builtin_is_constant_evaluated();
+#if __cplusplus >= 201103L
+ constexpr bool c = __builtin_is_constant_evaluated();
+#endif
+ float d = __builtin_is_constant_evaluated();
+ const float e = __builtin_is_constant_evaluated();
+ g(a, b
+#if __cplusplus >= 201103L
+ , c
+#endif
+ , d, e);
+}
+
+#else
+
+_Static_assert(b, "");
+#if __cplusplus >= 201103L
+static_assert(c, "");
+#endif
+_Static_assert(__builtin_constant_p(1) ? e == 1.0f : false, "");
+
+#endif
diff --git a/clang/test/SemaCXX/attr-require-constant-initialization.cpp b/clang/test/SemaCXX/attr-require-constant-initialization.cpp
index 32fc7f3eec99e..77a4d52538ddf 100644
--- a/clang/test/SemaCXX/attr-require-constant-initialization.cpp
+++ b/clang/test/SemaCXX/attr-require-constant-initialization.cpp
@@ -152,7 +152,7 @@ void test_basic_start_static_2_2() {
#else
ATTR static PODType pod; // expected-error {{variable does not have a constant initializer}}
// expected-note at -1 {{required by 'require_constant_initialization' attribute here}}
-// expected-note at -2 {{subobject of type 'int' is not initialized}}
+// expected-note-re at -2 {{{{non-constexpr constructor|subobject of type 'int' is not initialized}}}}
#endif
ATTR static PODType pot2 = {ReturnInt()}; // expected-error {{variable does not have a constant initializer}}
// expected-note at -1 {{required by 'require_constant_initialization' attribute here}}
@@ -191,7 +191,7 @@ struct TT2 {
PODType TT2::pod_noinit; // expected-note 0+ {{declared here}}
#if __cplusplus >= 201103L
// expected-error at -2 {{variable does not have a constant initializer}}
-// expected-note at -3 {{subobject of type 'int' is not initialized}}
+// expected-note-re at -3 {{{{non-constexpr constructor|subobject of type 'int' is not initialized}}}}
#endif
PODType TT2::pod_copy_init(TT2::pod_noinit); // expected-error {{variable does not have a constant initializer}}
#if __cplusplus >= 201103L
diff --git a/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp b/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
index 4af2b494751c9..c54d7c1b7aa84 100644
--- a/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
+++ b/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -Wno-constant-evaluated -triple=x86_64-linux-gnu
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
using size_t = decltype(sizeof(int));
namespace std {
@@ -119,3 +121,25 @@ struct TestConditionalExplicit {
};
TestConditionalExplicit e = 42;
#endif
+
+namespace fold_initializer {
+ // Global 'f' has a constant initializer.
+ const float f = __builtin_is_constant_evaluated();
+ static_assert(fold(f == 1.0f));
+
+ void g() {
+ // Local static 'sf' has a constant initializer.
+ static const float sf = __builtin_is_constant_evaluated();
+ static_assert(fold(sf == 1.0f));
+
+ // Local non-static 'f' has a non-constant initializer.
+ const float f = __builtin_is_constant_evaluated();
+ static_assert(fold(f == 0.0f));
+ }
+
+ struct A {
+ static const float f;
+ };
+ const float A::f = __builtin_is_constant_evaluated();
+ static_assert(fold(A::f == 1.0f));
+}
More information about the cfe-commits
mailing list