[PATCH] D53714: [AST] Only store the needed data in SwitchStmt.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345510: [AST] Only store the needed data in SwitchStmt
(authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D53714?vs=171337=171518#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53714
Files:
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/Import/switch-stmt/test.cpp
cfe/trunk/test/Misc/ast-dump-color.cpp
Index: cfe/trunk/include/clang/AST/Stmt.h
===
--- cfe/trunk/include/clang/AST/Stmt.h
+++ cfe/trunk/include/clang/AST/Stmt.h
@@ -177,6 +177,17 @@
unsigned : NumStmtBits;
+/// True if the SwitchStmt has storage for an init statement.
+unsigned HasInit : 1;
+
+/// True if the SwitchStmt has storage for a condition variable.
+unsigned HasVar : 1;
+
+/// If the SwitchStmt is a switch on an enum value, records whether all
+/// the enum values were covered by CaseStmts. The coverage information
+/// value is meant to be a hint for possible clients.
+unsigned AllEnumCasesCovered : 1;
+
/// The location of the "switch".
SourceLocation SwitchLoc;
};
@@ -1427,21 +1438,102 @@
};
/// SwitchStmt - This represents a 'switch' stmt.
-class SwitchStmt : public Stmt {
- enum { INIT, VAR, COND, BODY, END_EXPR };
- Stmt* SubExprs[END_EXPR];
+class SwitchStmt final : public Stmt,
+ private llvm::TrailingObjects {
+ friend TrailingObjects;
- // This points to a linked list of case and default statements and, if the
- // SwitchStmt is a switch on an enum value, records whether all the enum
- // values were covered by CaseStmts. The coverage information value is meant
- // to be a hint for possible clients.
- llvm::PointerIntPair FirstCase;
+ /// Points to a linked list of case and default statements.
+ SwitchCase *FirstCase;
-public:
- SwitchStmt(const ASTContext , Stmt *Init, VarDecl *Var, Expr *cond);
+ // SwitchStmt is followed by several trailing objects,
+ // some of which optional. Note that it would be more convenient to
+ // put the optional trailing objects at the end but this would change
+ // the order in children().
+ // The trailing objects are in order:
+ //
+ // * A "Stmt *" for the init statement.
+ //Present if and only if hasInitStorage().
+ //
+ // * A "Stmt *" for the condition variable.
+ //Present if and only if hasVarStorage(). This is in fact a "DeclStmt *".
+ //
+ // * A "Stmt *" for the condition.
+ //Always present. This is in fact an "Expr *".
+ //
+ // * A "Stmt *" for the body.
+ //Always present.
+ enum { InitOffset = 0, BodyOffsetFromCond = 1 };
+ enum { NumMandatoryStmtPtr = 2 };
+
+ unsigned numTrailingObjects(OverloadToken) const {
+return NumMandatoryStmtPtr + hasInitStorage() + hasVarStorage();
+ }
+
+ unsigned initOffset() const { return InitOffset; }
+ unsigned varOffset() const { return InitOffset + hasInitStorage(); }
+ unsigned condOffset() const {
+return InitOffset + hasInitStorage() + hasVarStorage();
+ }
+ unsigned bodyOffset() const { return condOffset() + BodyOffsetFromCond; }
+
+ /// Build a switch statement.
+ SwitchStmt(const ASTContext , Stmt *Init, VarDecl *Var, Expr *Cond);
/// Build a empty switch statement.
- explicit SwitchStmt(EmptyShell Empty) : Stmt(SwitchStmtClass, Empty) {}
+ explicit SwitchStmt(EmptyShell Empty, bool HasInit, bool HasVar);
+
+public:
+ /// Create a switch statement.
+ static SwitchStmt *Create(const ASTContext , Stmt *Init, VarDecl *Var,
+Expr *Cond);
+
+ /// Create an empty switch statement optionally with storage for
+ /// an init expression and a condition variable.
+ static SwitchStmt *CreateEmpty(const ASTContext , bool HasInit,
+ bool HasVar);
+
+ /// True if this SwitchStmt has storage for an init statement.
+ bool hasInitStorage() const { return SwitchStmtBits.HasInit; }
+
+ /// True if this SwitchStmt has storage for a condition variable.
+ bool hasVarStorage() const { return SwitchStmtBits.HasVar; }
+
+ Expr *getCond() {
+return reinterpret_cast(getTrailingObjects()[condOffset()]);
+ }
+
+ const Expr *getCond() const {
+return reinterpret_cast(getTrailingObjects()[condOffset()]);
+ }
+
+ void setCond(Expr *Cond) {
+getTrailingObjects()[condOffset()] = reinterpret_cast(Cond);
+ }
+
+ Stmt *getBody() { return getTrailingObjects()[bodyOffset()]; }
+ const Stmt *getBody() const {
+return getTrailingObjects()[bodyOffset()];
+ }
+
+ void setBody(Stmt *Body) {
+getTrailingObjects()[bodyOffset()] = Body;
+ }
+
+ Stmt
[PATCH] D53714: [AST] Only store the needed data in SwitchStmt.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D53714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53714: [AST] Only store the needed data in SwitchStmt.
riccibruno updated this revision to Diff 171337.
riccibruno added a reviewer: rsmith.
riccibruno added a comment.
Add a flag to the output of -ast-dump indicating which sub-statement
a `SwitchStmt` is storing. This removes the ambiguity in the output
of -ast-dump as per rsmith's comment in https://reviews.llvm.org/D53717.
Repository:
rC Clang
https://reviews.llvm.org/D53714
Files:
include/clang/AST/Stmt.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Stmt.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/Import/switch-stmt/test.cpp
test/Misc/ast-dump-color.cpp
Index: test/Misc/ast-dump-color.cpp
===
--- test/Misc/ast-dump-color.cpp
+++ test/Misc/ast-dump-color.cpp
@@ -46,7 +46,6 @@
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]FunctionDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:9:1[[RESET]], [[Yellow]]line:16:1[[RESET]]> [[Yellow]]line:9:6[[RESET]][[CYAN]] TestAttributedStmt[[RESET]] [[Green]]'void ()'[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| |-[[RESET]][[MAGENTA:.\[0;1;35m]]CompoundStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:27[[RESET]], [[Yellow]]line:16:1[[RESET]]>{{$}}
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]SwitchStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:3[[RESET]], [[Yellow]]line:15:3[[RESET]]>{{$}}
-//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[Blue:.\[0;34m]]<<>>[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:11[[RESET]]> [[Green]]'int'[[RESET]][[Cyan:.\[0;36m]][[RESET]][[Cyan]][[RESET]][[CYAN]] 1[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]CompoundStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:14[[RESET]], [[Yellow]]line:15:3[[RESET]]>{{$}}
//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:3[[RESET]], [[Yellow]]line:12:27[[RESET]]>{{$}}
Index: test/Import/switch-stmt/test.cpp
===
--- test/Import/switch-stmt/test.cpp
+++ test/Import/switch-stmt/test.cpp
@@ -1,8 +1,6 @@
// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: CompoundStmt
// CHECK-NEXT: CaseStmt
@@ -22,7 +20,6 @@
// CHECK-NEXT: DeclStmt
// CHECK-NEXT: VarDecl
// CHECK-SAME: varname
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: CompoundStmt
// CHECK-NEXT: CaseStmt
@@ -37,15 +34,11 @@
// CHECK-NEXT: BreakStmt
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: DefaultStmt
// CHECK-NEXT: BreakStmt
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: NullStmt
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -159,12 +159,22 @@
void ASTStmtWriter::VisitSwitchStmt(SwitchStmt *S) {
VisitStmt(S);
- Record.AddStmt(S->getInit());
- Record.AddDeclRef(S->getConditionVariable());
+
+ bool HasInit = S->getInit() != nullptr;
+ bool HasVar = S->getConditionVariableDeclStmt() != nullptr;
+ Record.push_back(HasInit);
+ Record.push_back(HasVar);
+ Record.push_back(S->isAllEnumCasesCovered());
+
Record.AddStmt(S->getCond());
Record.AddStmt(S->getBody());
+ if (HasInit)
+Record.AddStmt(S->getInit());
+ if (HasVar)
+Record.AddDeclRef(S->getConditionVariable());
+
Record.AddSourceLocation(S->getSwitchLoc());
- Record.push_back(S->isAllEnumCasesCovered());
+
for (SwitchCase *SC = S->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase())
Record.push_back(Writer.RecordSwitchCaseID(SC));
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -240,13 +240,21 @@
void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {
VisitStmt(S);
- S->setInit(Record.readSubStmt());
- S->setConditionVariable(Record.getContext(), ReadDeclAs());
+
+ bool HasInit = Record.readInt();
+ bool HasVar = Record.readInt();
+ bool AllEnumCasesCovered = Record.readInt();
+ if (AllEnumCasesCovered)
+S->setAllEnumCasesCovered();
+
S->setCond(Record.readSubExpr());
S->setBody(Record.readSubStmt());
+ if (HasInit)
+S->setInit(Record.readSubStmt());
+ if (HasVar)
+S->setConditionVariable(Record.getContext(), ReadDeclAs());
+
S->setSwitchLoc(ReadSourceLocation());
- if (Record.readInt())
-S->setAllEnumCasesCovered();
SwitchCase *PrevSC = nullptr;
[PATCH] D53714: [AST] Only store the needed data in SwitchStmt.
riccibruno created this revision.
riccibruno added a reviewer: rjmccall.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
Don't store the data for the init statement and condition variable
if not needed. This cuts the size of `SwitchStmt` by up to 2 pointers.
The order of the children is kept the same.
Also use the newly available space in the bit-fields of `Stmt`
to store the bit representing whether all enum have been covered
instead of using a `PointerIntPair`.
Repository:
rC Clang
https://reviews.llvm.org/D53714
Files:
include/clang/AST/Stmt.h
lib/AST/ASTImporter.cpp
lib/AST/Stmt.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/Import/switch-stmt/test.cpp
test/Misc/ast-dump-color.cpp
Index: test/Misc/ast-dump-color.cpp
===
--- test/Misc/ast-dump-color.cpp
+++ test/Misc/ast-dump-color.cpp
@@ -46,7 +46,6 @@
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]FunctionDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:9:1[[RESET]], [[Yellow]]line:16:1[[RESET]]> [[Yellow]]line:9:6[[RESET]][[CYAN]] TestAttributedStmt[[RESET]] [[Green]]'void ()'[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| |-[[RESET]][[MAGENTA:.\[0;1;35m]]CompoundStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:27[[RESET]], [[Yellow]]line:16:1[[RESET]]>{{$}}
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]SwitchStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:3[[RESET]], [[Yellow]]line:15:3[[RESET]]>{{$}}
-//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[Blue:.\[0;34m]]<<>>[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:11[[RESET]]> [[Green]]'int'[[RESET]][[Cyan:.\[0;36m]][[RESET]][[Cyan]][[RESET]][[CYAN]] 1[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]CompoundStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:14[[RESET]], [[Yellow]]line:15:3[[RESET]]>{{$}}
//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:3[[RESET]], [[Yellow]]line:12:27[[RESET]]>{{$}}
Index: test/Import/switch-stmt/test.cpp
===
--- test/Import/switch-stmt/test.cpp
+++ test/Import/switch-stmt/test.cpp
@@ -1,8 +1,6 @@
// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: CompoundStmt
// CHECK-NEXT: CaseStmt
@@ -22,7 +20,6 @@
// CHECK-NEXT: DeclStmt
// CHECK-NEXT: VarDecl
// CHECK-SAME: varname
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: CompoundStmt
// CHECK-NEXT: CaseStmt
@@ -37,15 +34,11 @@
// CHECK-NEXT: BreakStmt
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: DefaultStmt
// CHECK-NEXT: BreakStmt
// CHECK: SwitchStmt
-// CHECK-NEXT: <>
-// CHECK-NEXT: <>
// CHECK-NEXT: IntegerLiteral
// CHECK-NEXT: NullStmt
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -159,12 +159,22 @@
void ASTStmtWriter::VisitSwitchStmt(SwitchStmt *S) {
VisitStmt(S);
- Record.AddStmt(S->getInit());
- Record.AddDeclRef(S->getConditionVariable());
+
+ bool HasInit = !!S->getInit();
+ bool HasVar = !!S->getConditionVariableDeclStmt();
+ Record.push_back(HasInit);
+ Record.push_back(HasVar);
+ Record.push_back(S->isAllEnumCasesCovered());
+
Record.AddStmt(S->getCond());
Record.AddStmt(S->getBody());
+ if (HasInit)
+Record.AddStmt(S->getInit());
+ if (HasVar)
+Record.AddDeclRef(S->getConditionVariable());
+
Record.AddSourceLocation(S->getSwitchLoc());
- Record.push_back(S->isAllEnumCasesCovered());
+
for (SwitchCase *SC = S->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase())
Record.push_back(Writer.RecordSwitchCaseID(SC));
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -240,13 +240,21 @@
void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {
VisitStmt(S);
- S->setInit(Record.readSubStmt());
- S->setConditionVariable(Record.getContext(), ReadDeclAs());
+
+ bool HasInit = Record.readInt();
+ bool HasVar = Record.readInt();
+ bool AllEnumCasesCovered = Record.readInt();
+ if (AllEnumCasesCovered)
+S->setAllEnumCasesCovered();
+
S->setCond(Record.readSubExpr());
S->setBody(Record.readSubStmt());
+ if (HasInit)
+S->setInit(Record.readSubStmt());
+ if (HasVar)
+S->setConditionVariable(Record.getContext(), ReadDeclAs());
+
