[clang] e7ce052 - [clang][Serialization] Fix the serialization of ConstantExpr.
Bruno Ricci via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 21 06:03:03 PDT 2020
Author: Bruno Ricci
Date: 2020-06-21T13:59:10+01:00
New Revision: e7ce0528202306d8b751f132d9d3a6519ce4e688
URL: https://github.com/llvm/llvm-project/commit/e7ce0528202306d8b751f132d9d3a6519ce4e688
DIFF: https://github.com/llvm/llvm-project/commit/e7ce0528202306d8b751f132d9d3a6519ce4e688.diff
LOG: [clang][Serialization] Fix the serialization of ConstantExpr.
The serialization of ConstantExpr has currently a number of problems:
- Some fields are just not serialized (ConstantExprBits.APValueKind and
ConstantExprBits.IsImmediateInvocation).
- ASTStmtReader::VisitConstantExpr forgets to add the trailing APValue
to the list of objects to be destroyed when the APValue needs cleanup.
While we are at it, bring the serialization of ConstantExpr more in-line
with what is done with the other expressions by doing the following NFCs:
- Get rid of ConstantExpr::DefaultInit. It is better to not initialize
the fields of an empty ConstantExpr since this will allow msan to
detect if a field was not deserialized.
- Move the initialization of the fields of ConstantExpr to the constructor;
ConstantExpr::Create allocates the memory and ConstantExpr::ConstantExpr
is responsible for the initialization.
Review after commit since this is a straightforward mechanical fix
similar to the other serialization fixes.
Added:
clang/test/AST/ast-dump-constant-expr.cpp
Modified:
clang/include/clang/AST/Expr.h
clang/include/clang/AST/Stmt.h
clang/lib/AST/Expr.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index f8f104b5580b..8ee8aa96ae63 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -990,6 +990,9 @@ class ConstantExpr final
static_assert(std::is_same<uint64_t, llvm::APInt::WordType>::value,
"ConstantExpr assumes that llvm::APInt::WordType is uint64_t "
"for tail-allocated storage");
+ friend TrailingObjects;
+ friend class ASTStmtReader;
+ friend class ASTStmtWriter;
public:
/// Describes the kind of result that can be tail-allocated.
@@ -1003,7 +1006,6 @@ class ConstantExpr final
return ConstantExprBits.ResultKind == ConstantExpr::RSK_Int64;
}
- void DefaultInit(ResultStorageKind StorageKind);
uint64_t &Int64Result() {
assert(ConstantExprBits.ResultKind == ConstantExpr::RSK_Int64 &&
"invalid accessor");
@@ -1021,21 +1023,18 @@ class ConstantExpr final
return const_cast<ConstantExpr *>(this)->APValueResult();
}
- ConstantExpr(Expr *subexpr, ResultStorageKind StorageKind);
- ConstantExpr(ResultStorageKind StorageKind, EmptyShell Empty);
+ ConstantExpr(Expr *SubExpr, ResultStorageKind StorageKind,
+ bool IsImmediateInvocation);
+ ConstantExpr(EmptyShell Empty, ResultStorageKind StorageKind);
public:
- friend TrailingObjects;
- friend class ASTStmtReader;
- friend class ASTStmtWriter;
static ConstantExpr *Create(const ASTContext &Context, Expr *E,
const APValue &Result);
static ConstantExpr *Create(const ASTContext &Context, Expr *E,
ResultStorageKind Storage = RSK_None,
bool IsImmediateInvocation = false);
static ConstantExpr *CreateEmpty(const ASTContext &Context,
- ResultStorageKind StorageKind,
- EmptyShell Empty);
+ ResultStorageKind StorageKind);
static ResultStorageKind getStorageKind(const APValue &Value);
static ResultStorageKind getStorageKind(const Type *T,
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index d76d99b65551..a226790aa76c 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -325,7 +325,7 @@ class alignas(void *) Stmt {
/// The kind of result that is tail-allocated.
unsigned ResultKind : 2;
- /// The kind of Result as defined by APValue::Kind
+ /// The kind of Result as defined by APValue::Kind.
unsigned APValueKind : 4;
/// When ResultKind == RSK_Int64, true if the tail-allocated integer is
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 27a12b880a7b..5cbd66f11601 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -244,6 +244,7 @@ static void AssertResultStorageKind(ConstantExpr::ResultStorageKind Kind) {
assert((Kind == ConstantExpr::RSK_APValue ||
Kind == ConstantExpr::RSK_Int64 || Kind == ConstantExpr::RSK_None) &&
"Invalid StorageKind Value");
+ (void)Kind;
}
ConstantExpr::ResultStorageKind
@@ -268,33 +269,31 @@ ConstantExpr::getStorageKind(const Type *T, const ASTContext &Context) {
return ConstantExpr::RSK_APValue;
}
-void ConstantExpr::DefaultInit(ResultStorageKind StorageKind) {
+ConstantExpr::ConstantExpr(Expr *SubExpr, ResultStorageKind StorageKind,
+ bool IsImmediateInvocation)
+ : FullExpr(ConstantExprClass, SubExpr) {
ConstantExprBits.ResultKind = StorageKind;
ConstantExprBits.APValueKind = APValue::None;
+ ConstantExprBits.IsUnsigned = false;
+ ConstantExprBits.BitWidth = 0;
ConstantExprBits.HasCleanup = false;
- ConstantExprBits.IsImmediateInvocation = false;
+ ConstantExprBits.IsImmediateInvocation = IsImmediateInvocation;
+
if (StorageKind == ConstantExpr::RSK_APValue)
::new (getTrailingObjects<APValue>()) APValue();
}
-ConstantExpr::ConstantExpr(Expr *subexpr, ResultStorageKind StorageKind)
- : FullExpr(ConstantExprClass, subexpr) {
- DefaultInit(StorageKind);
-}
-
ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
ResultStorageKind StorageKind,
bool IsImmediateInvocation) {
assert(!isa<ConstantExpr>(E));
AssertResultStorageKind(StorageKind);
+
unsigned Size = totalSizeToAlloc<APValue, uint64_t>(
StorageKind == ConstantExpr::RSK_APValue,
StorageKind == ConstantExpr::RSK_Int64);
void *Mem = Context.Allocate(Size, alignof(ConstantExpr));
- ConstantExpr *Self = new (Mem) ConstantExpr(E, StorageKind);
- Self->ConstantExprBits.IsImmediateInvocation =
- IsImmediateInvocation;
- return Self;
+ return new (Mem) ConstantExpr(E, StorageKind, IsImmediateInvocation);
}
ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
@@ -305,21 +304,23 @@ ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E,
return Self;
}
-ConstantExpr::ConstantExpr(ResultStorageKind StorageKind, EmptyShell Empty)
+ConstantExpr::ConstantExpr(EmptyShell Empty, ResultStorageKind StorageKind)
: FullExpr(ConstantExprClass, Empty) {
- DefaultInit(StorageKind);
+ ConstantExprBits.ResultKind = StorageKind;
+
+ if (StorageKind == ConstantExpr::RSK_APValue)
+ ::new (getTrailingObjects<APValue>()) APValue();
}
ConstantExpr *ConstantExpr::CreateEmpty(const ASTContext &Context,
- ResultStorageKind StorageKind,
- EmptyShell Empty) {
+ ResultStorageKind StorageKind) {
AssertResultStorageKind(StorageKind);
+
unsigned Size = totalSizeToAlloc<APValue, uint64_t>(
StorageKind == ConstantExpr::RSK_APValue,
StorageKind == ConstantExpr::RSK_Int64);
void *Mem = Context.Allocate(Size, alignof(ConstantExpr));
- ConstantExpr *Self = new (Mem) ConstantExpr(StorageKind, Empty);
- return Self;
+ return new (Mem) ConstantExpr(EmptyShell(), StorageKind);
}
void ConstantExpr::MoveIntoResult(APValue &Value, const ASTContext &Context) {
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index d5ae8979b1e1..0c379839ba83 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -541,18 +541,35 @@ void ASTStmtReader::VisitExpr(Expr *E) {
void ASTStmtReader::VisitConstantExpr(ConstantExpr *E) {
VisitExpr(E);
- E->ConstantExprBits.ResultKind = Record.readInt();
- switch (E->ConstantExprBits.ResultKind) {
- case ConstantExpr::RSK_Int64: {
+
+ auto StorageKind = Record.readInt();
+ assert(E->ConstantExprBits.ResultKind == StorageKind && "Wrong ResultKind!");
+
+ E->ConstantExprBits.APValueKind = Record.readInt();
+ E->ConstantExprBits.IsUnsigned = Record.readInt();
+ E->ConstantExprBits.BitWidth = Record.readInt();
+ E->ConstantExprBits.HasCleanup = false; // Not serialized, see below.
+ E->ConstantExprBits.IsImmediateInvocation = Record.readInt();
+
+ switch (StorageKind) {
+ case ConstantExpr::RSK_None:
+ break;
+
+ case ConstantExpr::RSK_Int64:
E->Int64Result() = Record.readInt();
- uint64_t tmp = Record.readInt();
- E->ConstantExprBits.IsUnsigned = tmp & 0x1;
- E->ConstantExprBits.BitWidth = tmp >> 1;
break;
- }
+
case ConstantExpr::RSK_APValue:
E->APValueResult() = Record.readAPValue();
+ if (E->APValueResult().needsCleanup()) {
+ E->ConstantExprBits.HasCleanup = true;
+ Record.getContext().addDestruction(&E->APValueResult());
+ }
+ break;
+ default:
+ llvm_unreachable("unexpected ResultKind!");
}
+
E->setSubExpr(Record.readSubExpr());
}
@@ -2859,10 +2876,8 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
case EXPR_CONSTANT:
S = ConstantExpr::CreateEmpty(
- Context,
- static_cast<ConstantExpr::ResultStorageKind>(
- Record[ASTStmtReader::NumExprFields]),
- Empty);
+ Context, static_cast<ConstantExpr::ResultStorageKind>(
+ /*StorageKind=*/Record[ASTStmtReader::NumExprFields]));
break;
case EXPR_PREDEFINED:
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 3a3b7bdfbb46..b13d0df9c376 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -548,16 +548,27 @@ void ASTStmtWriter::VisitExpr(Expr *E) {
void ASTStmtWriter::VisitConstantExpr(ConstantExpr *E) {
VisitExpr(E);
- Record.push_back(static_cast<uint64_t>(E->ConstantExprBits.ResultKind));
+ Record.push_back(E->ConstantExprBits.ResultKind);
+
+ Record.push_back(E->ConstantExprBits.APValueKind);
+ Record.push_back(E->ConstantExprBits.IsUnsigned);
+ Record.push_back(E->ConstantExprBits.BitWidth);
+ // HasCleanup not serialized since we can just query the APValue.
+ Record.push_back(E->ConstantExprBits.IsImmediateInvocation);
+
switch (E->ConstantExprBits.ResultKind) {
+ case ConstantExpr::RSK_None:
+ break;
case ConstantExpr::RSK_Int64:
Record.push_back(E->Int64Result());
- Record.push_back(E->ConstantExprBits.IsUnsigned |
- E->ConstantExprBits.BitWidth << 1);
break;
case ConstantExpr::RSK_APValue:
Record.AddAPValue(E->APValueResult());
+ break;
+ default:
+ llvm_unreachable("unexpected ResultKind!");
}
+
Record.AddStmt(E->getSubExpr());
Code = serialization::EXPR_CONSTANT;
}
diff --git a/clang/test/AST/ast-dump-constant-expr.cpp b/clang/test/AST/ast-dump-constant-expr.cpp
new file mode 100644
index 000000000000..c9ddb245ef64
--- /dev/null
+++ b/clang/test/AST/ast-dump-constant-expr.cpp
@@ -0,0 +1,80 @@
+// Test without serialization:
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -ast-dump -ast-dump-filter Test %s \
+// RUN: | FileCheck --strict-whitespace --match-full-lines %s
+//
+// Test with serialization:
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown -include-pch %t \
+// RUN: -ast-dump-all -ast-dump-filter Test /dev/null \
+// RUN: | FileCheck --strict-whitespace --match-full-lines %s
+
+// FIXME: ASTRecordReader::readAPValue and ASTRecordWriter::AddAPValue
+// just give up on some APValue kinds! This *really* should be fixed.
+
+struct array_holder {
+ int i[2];
+};
+
+struct S {
+ int i = 42;
+};
+
+union U {
+ int i = 42;
+ float f;
+};
+
+struct SU {
+ S s[2];
+ U u[3];
+};
+
+consteval int test_Int() { return 42; }
+consteval float test_Float() { return 1.0f; }
+consteval _Complex int test_ComplexInt() { return 1+2i; }
+consteval _Complex float test_ComplexFloat() { return 1.2f+3.4fi; }
+consteval __int128 test_Int128() { return (__int128)0xFFFFFFFFFFFFFFFF + (__int128)1; }
+// FIXME: consteval array_holder test_Array() { return array_holder(); }
+// FIXME: consteval S test_Struct() { return S(); }
+// FIXME: consteval U test_Union() { return U(); }
+// FIXME: consteval SU test_SU() { return SU(); }
+
+void Test() {
+ (void) test_Int();
+ (void) test_Float();
+ (void) test_ComplexInt();
+ (void) test_ComplexFloat();
+ (void) test_Int128();
+ //(void) test_Array();
+ //(void) test_Struct();
+ //(void) test_Union();
+ //(void) test_SU();
+}
+// CHECK:Dumping Test:
+// CHECK-NEXT:FunctionDecl {{.*}} <{{.*}}ast-dump-constant-expr.cpp:42:1, line:52:1> line:42:6{{( imported)?}} Test 'void ()'
+// CHECK-NEXT:`-CompoundStmt {{.*}} <col:13, line:52:1>
+// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:43:3, col:19> 'void' <ToVoid>
+// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:19> 'int' Int: 42
+// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:19> 'int'
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> 'int (*)()' <FunctionToPointerDecay>
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> 'int ()' lvalue Function {{.*}} 'test_Int' 'int ()'
+// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:44:3, col:21> 'void' <ToVoid>
+// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:21> 'float' Float: 1.000000e+00
+// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:21> 'float'
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> 'float (*)()' <FunctionToPointerDecay>
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> 'float ()' lvalue Function {{.*}} 'test_Float' 'float ()'
+// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:45:3, col:26> 'void' <ToVoid>
+// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:26> '_Complex int' ComplexInt: 1, 2
+// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:26> '_Complex int'
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> '_Complex int (*)()' <FunctionToPointerDecay>
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> '_Complex int ()' lvalue Function {{.*}} 'test_ComplexInt' '_Complex int ()'
+// CHECK-NEXT: |-CStyleCastExpr {{.*}} <line:46:3, col:28> 'void' <ToVoid>
+// CHECK-NEXT: | `-ConstantExpr {{.*}} <col:10, col:28> '_Complex float' ComplexFloat: 1.200000e+00, 3.400000e+00
+// CHECK-NEXT: | `-CallExpr {{.*}} <col:10, col:28> '_Complex float'
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} <col:10> '_Complex float (*)()' <FunctionToPointerDecay>
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} <col:10> '_Complex float ()' lvalue Function {{.*}} 'test_ComplexFloat' '_Complex float ()'
+// CHECK-NEXT: `-CStyleCastExpr {{.*}} <line:47:3, col:22> 'void' <ToVoid>
+// CHECK-NEXT: `-ConstantExpr {{.*}} <col:10, col:22> '__int128' Int: 18446744073709551616
+// CHECK-NEXT: `-CallExpr {{.*}} <col:10, col:22> '__int128'
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <col:10> '__int128 (*)()' <FunctionToPointerDecay>
+// CHECK-NEXT: `-DeclRefExpr {{.*}} <col:10> '__int128 ()' lvalue Function {{.*}} 'test_Int128' '__int128 ()'
More information about the cfe-commits
mailing list