Add Make factory function to Block.
Block::Make always makes a real Block object. This is important because
many callers rely on Blocks specifically; e.g. a function body must be a
scoped Block, nothing else will do.
However, unscoped Blocks are just a collection of Statements and usually
have more flexibility. So, Block::MakeUnscoped is a factory function
that will return the Statement as-is for a single-statement unscoped
Block. For an entirely empty Block, MakeUnscoped returns Nop.
Change-Id: Ied65d505bde4ea7f6157a039944018546e4b7333
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384180
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl.gni b/gn/sksl.gni
index 2493bcc..582e176 100644
--- a/gn/sksl.gni
+++ b/gn/sksl.gni
@@ -88,6 +88,7 @@
"$_src/sksl/dsl/priv/DSLWriter.h",
"$_src/sksl/ir/SkSLBinaryExpression.cpp",
"$_src/sksl/ir/SkSLBinaryExpression.h",
+ "$_src/sksl/ir/SkSLBlock.cpp",
"$_src/sksl/ir/SkSLBlock.h",
"$_src/sksl/ir/SkSLBoolLiteral.h",
"$_src/sksl/ir/SkSLBreakStatement.h",
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index a975dd2..31da420 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -1385,6 +1385,7 @@
}
}
fIndentation--;
+ this->finishLine();
this->write("}");
}
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 9c32dfe..f877750 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -181,8 +181,8 @@
statements.reserve_back(2);
statements.push_back(getNormalizeSkPositionCode());
statements.push_back(std::move(result));
- return std::make_unique<Block>(statement.fOffset, std::move(statements),
- fSymbolTable);
+ return Block::Make(statement.fOffset, std::move(statements),
+ fSymbolTable, /*isScope=*/true);
}
}
}
@@ -201,7 +201,7 @@
}
statements.push_back(std::move(statement));
}
- return std::make_unique<Block>(block.fOffset, std::move(statements), fSymbolTable);
+ return Block::Make(block.fOffset, std::move(statements), fSymbolTable);
}
std::unique_ptr<Statement> IRGenerator::convertVarDeclarationStatement(const ASTNode& s) {
@@ -210,12 +210,7 @@
if (decls.empty()) {
return nullptr;
}
- if (decls.size() == 1) {
- return std::move(decls.front());
- } else {
- return std::make_unique<Block>(s.fOffset, std::move(decls), /*symbols=*/nullptr,
- /*isScope=*/false);
- }
+ return Block::MakeUnscoped(s.fOffset, std::move(decls));
}
int IRGenerator::convertArraySize(const Type& type, int offset, const ASTNode& s) {
@@ -668,11 +663,7 @@
}
++childIter;
- auto caseBlock = std::make_unique<Block>(c.fOffset,
- StatementArray{},
- /*symbols=*/nullptr,
- /*isScope=*/false);
- StatementArray& statements = caseBlock->children();
+ StatementArray statements;
for (; childIter != c.end(); ++childIter) {
std::unique_ptr<Statement> converted = this->convertStatement(*childIter);
if (!converted) {
@@ -680,7 +671,8 @@
}
statements.push_back(std::move(converted));
}
- caseStatements.push_back(std::move(caseBlock));
+
+ caseStatements.push_back(Block::MakeUnscoped(c.fOffset, std::move(statements)));
}
return SwitchStatement::Convert(fContext, s.fOffset, s.getBool(), std::move(value),
std::move(caseValues), std::move(caseStatements), fSymbolTable);
@@ -776,12 +768,15 @@
Token::Kind::TK_EQ,
IntLiteral::Make(fContext, /*offset=*/-1, /*value=*/0));
auto initializer = ExpressionStatement::Make(fContext, std::move(assignment));
- auto loop = ForStatement::Make(
- fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next),
- std::make_unique<Block>(/*offset=*/-1, std::move(loopBody)), fSymbolTable);
+ auto loop = ForStatement::Make(fContext, /*offset=*/-1,
+ std::move(initializer),
+ std::move(test),
+ std::move(next),
+ Block::Make(/*offset=*/-1, std::move(loopBody)),
+ fSymbolTable);
StatementArray children;
children.push_back(std::move(loop));
- return std::make_unique<Block>(/*offset=*/-1, std::move(children));
+ return Block::Make(/*offset=*/-1, std::move(children));
}
std::unique_ptr<Statement> IRGenerator::getNormalizeSkPositionCode() {
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 0d42f03..011c443 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -429,9 +429,9 @@
switch (statement.kind()) {
case Statement::Kind::kBlock: {
const Block& b = statement.as<Block>();
- return std::make_unique<Block>(offset, blockStmts(b),
- SymbolTable::WrapIfBuiltin(b.symbolTable()),
- b.isScope());
+ return Block::Make(offset, blockStmts(b),
+ SymbolTable::WrapIfBuiltin(b.symbolTable()),
+ b.isScope());
}
case Statement::Kind::kBreak:
@@ -506,8 +506,7 @@
block.reserve_back(2);
block.push_back(std::move(assignment));
block.push_back(ContinueStatement::Make(offset));
- return std::make_unique<Block>(offset, std::move(block), /*symbols=*/nullptr,
- /*isScope=*/true);
+ return Block::Make(offset, std::move(block), /*symbols=*/nullptr, /*isScope=*/true);
}
// Functions without early returns aren't wrapped in a for loop and don't need to worry
// about breaking out of the control flow.
@@ -623,19 +622,14 @@
bool hasEarlyReturn = (returnComplexity >= ReturnComplexity::kEarlyReturns);
InlinedCall inlinedCall;
- inlinedCall.fInlinedBody = std::make_unique<Block>(offset, StatementArray{},
- /*symbols=*/nullptr,
- /*isScope=*/false);
+ StatementArray inlinedBlockStmts;
+ inlinedBlockStmts.reserve_back(1 + // Inline marker
+ 1 + // Result variable
+ arguments.size() + // Function arguments (passing in)
+ arguments.size() + // Function arguments (copy out-params back)
+ 1); // Block for inlined code
- Block& inlinedBody = *inlinedCall.fInlinedBody;
- inlinedBody.children().reserve_back(
- 1 + // Inline marker
- 1 + // Result variable
- arguments.size() + // Function arguments (passing in)
- arguments.size() + // Function arguments (copy out-params back)
- 1); // Block for inlined code
-
- inlinedBody.children().push_back(InlineMarker::Make(&call->function()));
+ inlinedBlockStmts.push_back(InlineMarker::Make(&call->function()));
std::unique_ptr<Expression> resultExpr;
if (returnComplexity > ReturnComplexity::kSingleSafeReturn &&
@@ -648,7 +642,7 @@
&function.declaration().returnType(),
symbolTable.get(), Modifiers{},
caller->isBuiltin(), &noInitialValue);
- inlinedBody.children().push_back(std::move(var.fVarDecl));
+ inlinedBlockStmts.push_back(std::move(var.fVarDecl));
resultExpr = std::make_unique<VariableReference>(/*offset=*/-1, var.fVarSymbol);
}
@@ -675,7 +669,7 @@
InlineVariable var = this->makeInlineVariable(param->name(), &arguments[i]->type(),
symbolTable.get(), param->modifiers(),
caller->isBuiltin(), &arguments[i]);
- inlinedBody.children().push_back(std::move(var.fVarDecl));
+ inlinedBlockStmts.push_back(std::move(var.fVarDecl));
varMap[param] = std::make_unique<VariableReference>(/*offset=*/-1, var.fVarSymbol);
}
@@ -713,20 +707,20 @@
Token::Kind::TK_PLUSPLUS);
// {...}
- auto innerBlock = std::make_unique<Block>(offset, StatementArray{},
- /*symbols=*/nullptr, /*isScope=*/true);
+ auto innerBlock = Block::Make(offset, StatementArray{},
+ /*symbols=*/nullptr, /*isScope=*/true);
inlineStatements = &innerBlock->children();
// for (int _1_loop = 0; _1_loop < 1; _1_loop++) {...}
- inlinedBody.children().push_back(ForStatement::Make(*fContext, /*offset=*/-1,
- std::move(loopVar.fVarDecl),
- std::move(test),
- std::move(increment),
- std::move(innerBlock),
- symbolTable));
+ inlinedBlockStmts.push_back(ForStatement::Make(*fContext, /*offset=*/-1,
+ std::move(loopVar.fVarDecl),
+ std::move(test),
+ std::move(increment),
+ std::move(innerBlock),
+ symbolTable));
} else {
// No early returns, so we can just dump the code into our existing scopeless block.
- inlineStatements = &inlinedBody.children();
+ inlineStatements = &inlinedBlockStmts;
}
inlineStatements->reserve_back(body.children().size() + argsToCopyBack.size());
@@ -748,6 +742,11 @@
std::move(varMap[p]))));
}
+ // Wrap all of the generated statements in a block. We need a real Block here, so we can't use
+ // MakeUnscoped. This is because we need to add another child statement to the Block later.
+ inlinedCall.fInlinedBody = Block::Make(offset, std::move(inlinedBlockStmts),
+ /*symbols=*/nullptr, /*isScope=*/false);
+
if (resultExpr) {
// Return our result expression as-is.
inlinedCall.fReplacementExpr = std::move(resultExpr);
diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp
index c3bd292..5e5d413 100644
--- a/src/sksl/SkSLRehydrator.cpp
+++ b/src/sksl/SkSLRehydrator.cpp
@@ -351,8 +351,7 @@
statements.push_back(this->statement());
}
bool isScope = this->readU8();
- return std::make_unique<Block>(/*offset=*/-1, std::move(statements), fSymbolTable,
- isScope);
+ return Block::Make(/*offset=*/-1, std::move(statements), fSymbolTable, isScope);
}
case Rehydrator::kBreak_Command:
return BreakStatement::Make(/*offset=*/-1);
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 8b7b662..5f9fce1 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -3262,8 +3262,8 @@
// Function bodies are always wrapped in a Block.
StatementArray entrypointStmts;
entrypointStmts.push_back(std::move(assignmentStmt));
- auto entrypointBlock = std::make_unique<Block>(/*offset=*/-1, std::move(entrypointStmts),
- symbolTable, /*isScope=*/true);
+ auto entrypointBlock = Block::Make(/*offset=*/-1, std::move(entrypointStmts),
+ symbolTable, /*isScope=*/true);
// Declare an entrypoint function.
EntrypointAdapter adapter;
adapter.fLayout = {};
diff --git a/src/sksl/generated/sksl_gpu.dehydrated.sksl b/src/sksl/generated/sksl_gpu.dehydrated.sksl
index 797d444..dfb5975 100644
--- a/src/sksl/generated/sksl_gpu.dehydrated.sksl
+++ b/src/sksl/generated/sksl_gpu.dehydrated.sksl
@@ -5619,273 +5619,213 @@
49,16,4,0,30,
28,
40,1,0,0,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,154,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,1,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,157,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,2,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,160,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,3,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,163,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,4,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,166,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,5,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,169,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,6,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,172,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,7,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,175,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,8,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,178,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,9,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,181,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,10,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,184,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,11,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,187,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,12,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,190,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,13,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,193,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,14,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,196,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,15,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,202,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,16,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,205,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,17,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,208,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,18,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,221,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,19,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,227,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,20,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,230,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,21,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,236,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,22,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,239,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,23,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,242,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,24,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,245,3,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,25,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,6,4,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,26,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,9,4,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,27,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,12,4,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
28,
40,1,0,28,0,0,0,
-2,
-50,1,
34,
21,
40,15,2,15,4,2,
49,17,4,0,
-49,18,4,0,0,
+49,18,4,0,
50,
-2,
-50,1,
34,
6,
40,15,2,1,
19,
-40,176,0,0,0,0,0,0,1,29,154,3,157,3,160,3,163,3,166,3,169,3,172,3,175,3,178,3,181,3,184,3,187,3,190,3,193,3,196,3,202,3,205,3,208,3,221,3,227,3,230,3,236,3,239,3,242,3,245,3,6,4,9,4,12,4,15,4,
+40,176,0,0,0,0,0,1,29,154,3,157,3,160,3,163,3,166,3,169,3,172,3,175,3,178,3,181,3,184,3,187,3,190,3,193,3,196,3,202,3,205,3,208,3,221,3,227,3,230,3,236,3,239,3,242,3,245,3,6,4,9,4,12,4,15,4,
22,21,4,
2,
42,0,0,0,0,1,
diff --git a/src/sksl/ir/SkSLBlock.cpp b/src/sksl/ir/SkSLBlock.cpp
new file mode 100644
index 0000000..82238e8
--- /dev/null
+++ b/src/sksl/ir/SkSLBlock.cpp
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/sksl/ir/SkSLBlock.h"
+#include "src/sksl/ir/SkSLNop.h"
+
+#include <iterator>
+
+namespace SkSL {
+
+static void eliminate_empty_statements(StatementArray* statements) {
+ // Remove all the statements which are empty.
+ std::unique_ptr<Statement>* iter = std::remove_if(
+ statements->begin(), statements->end(), [](const std::unique_ptr<Statement>& stmt) {
+ return stmt->isEmpty();
+ });
+
+ // Shrink the statement array to match.
+ statements->resize(std::distance(statements->begin(), iter));
+}
+
+std::unique_ptr<Statement> Block::MakeUnscoped(int offset, StatementArray statements) {
+ eliminate_empty_statements(&statements);
+ if (statements.empty()) {
+ return Nop::Make();
+ }
+ if (statements.size() == 1) {
+ return std::move(statements.front());
+ }
+ return std::make_unique<Block>(offset, std::move(statements),
+ /*symbols=*/nullptr, /*isScope=*/false);
+}
+
+std::unique_ptr<Block> Block::Make(int offset,
+ StatementArray statements,
+ std::shared_ptr<SymbolTable> symbols,
+ bool isScope) {
+ // Nothing to optimize here--eliminate_empty_statements doesn't actually improve the generated
+ // code, and we promise to return a Block.
+ return std::make_unique<Block>(offset, std::move(statements), std::move(symbols), isScope);
+}
+
+std::unique_ptr<Statement> Block::clone() const {
+ StatementArray cloned;
+ cloned.reserve_back(this->children().size());
+ for (const std::unique_ptr<Statement>& stmt : this->children()) {
+ cloned.push_back(stmt->clone());
+ }
+ return std::make_unique<Block>(fOffset,
+ std::move(cloned),
+ SymbolTable::WrapIfBuiltin(this->symbolTable()),
+ this->isScope());
+}
+
+String Block::description() const {
+ String result;
+ if (fIsScope) {
+ result += "{";
+ }
+ for (const std::unique_ptr<Statement>& stmt : this->children()) {
+ result += "\n";
+ result += stmt->description();
+ }
+ result += fIsScope ? "\n}\n" : "\n";
+ return result;
+}
+
+} // namespace SkSL
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index a59e2ea..fba414f 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -27,6 +27,17 @@
, fSymbolTable(std::move(symbols))
, fIsScope(isScope) {}
+ // Make always makes a real Block object. This is important because many callers rely on Blocks
+ // specifically; e.g. a function body must be a scoped Block, nothing else will do.
+ static std::unique_ptr<Block> Make(int offset,
+ StatementArray statements,
+ std::shared_ptr<SymbolTable> symbols = nullptr,
+ bool isScope = true);
+
+ // An unscoped Block is just a collection of Statements. For a single-statement Block,
+ // MakeUnscoped will return the Statement as-is. For an empty Block, MakeUnscoped returns Nop.
+ static std::unique_ptr<Statement> MakeUnscoped(int offset, StatementArray statements);
+
const StatementArray& children() const {
return fChildren;
}
@@ -56,36 +67,16 @@
return true;
}
- std::unique_ptr<Statement> clone() const override {
- StatementArray cloned;
- cloned.reserve_back(this->children().size());
- for (const std::unique_ptr<Statement>& stmt : this->children()) {
- cloned.push_back(stmt->clone());
- }
- return std::make_unique<Block>(fOffset, std::move(cloned),
- SymbolTable::WrapIfBuiltin(this->symbolTable()),
- this->isScope());
- }
+ std::unique_ptr<Statement> clone() const override;
- String description() const override {
- String result;
- if (fIsScope) {
- result += "{";
- }
- for (const std::unique_ptr<Statement>& stmt : this->children()) {
- result += "\n";
- result += stmt->description();
- }
- result += fIsScope ? "\n}\n" : "\n";
- return result;
- }
+ String description() const override;
private:
StatementArray fChildren;
std::shared_ptr<SymbolTable> fSymbolTable;
- // if isScope is false, this is just a group of statements rather than an actual
- // language-level block. This allows us to pass around multiple statements as if they were a
- // single unit, with no semantic impact.
+ // If isScope is false, this is just a group of statements rather than an actual language-level
+ // block. This allows us to pass around multiple statements as if they were a single unit, with
+ // no semantic impact.
bool fIsScope;
using INHERITED = Statement;
diff --git a/src/sksl/ir/SkSLSwitchStatement.cpp b/src/sksl/ir/SkSLSwitchStatement.cpp
index e70bee3..1a4630f 100644
--- a/src/sksl/ir/SkSLSwitchStatement.cpp
+++ b/src/sksl/ir/SkSLSwitchStatement.cpp
@@ -93,8 +93,8 @@
move_all_but_break(stmt, &blockStmts);
}
- target->push_back(std::make_unique<Block>(block.fOffset, std::move(blockStmts),
- block.symbolTable(), block.isScope()));
+ target->push_back(Block::Make(block.fOffset, std::move(blockStmts),
+ block.symbolTable(), block.isScope()));
break;
}
@@ -161,8 +161,7 @@
}
// Return our newly-synthesized block.
- return std::make_unique<Block>(caseToCapture->fOffset, std::move(caseStmts),
- std::move(symbolTable));
+ return Block::Make(caseToCapture->fOffset, std::move(caseStmts), std::move(symbolTable));
}
std::unique_ptr<Statement> SwitchStatement::Convert(const Context& context,