Add Make factory functions to basic statements.
These are the simple cases like `break` and `continue` which don't have
any optimization opportunities, types to coerce, or errors to report.
We now have Make functions for consistency across our IRNodes, but they
don't contain any interesting logic.
Change-Id: Iefdb8e3dad66f131dc892f4a7dc647246bf2554e
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383707
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index d285f0e..053eba4 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -714,11 +714,7 @@
std::unique_ptr<Statement> IRGenerator::convertReturn(int offset,
std::unique_ptr<Expression> result) {
- if (result) {
- return std::make_unique<ReturnStatement>(std::move(result));
- } else {
- return std::make_unique<ReturnStatement>(offset);
- }
+ return ReturnStatement::Make(offset, std::move(result));
}
std::unique_ptr<Statement> IRGenerator::convertReturn(const ASTNode& r) {
@@ -736,12 +732,12 @@
std::unique_ptr<Statement> IRGenerator::convertBreak(const ASTNode& b) {
SkASSERT(b.fKind == ASTNode::Kind::kBreak);
- return std::make_unique<BreakStatement>(b.fOffset);
+ return BreakStatement::Make(b.fOffset);
}
std::unique_ptr<Statement> IRGenerator::convertContinue(const ASTNode& c) {
SkASSERT(c.fKind == ASTNode::Kind::kContinue);
- return std::make_unique<ContinueStatement>(c.fOffset);
+ return ContinueStatement::Make(c.fOffset);
}
std::unique_ptr<Statement> IRGenerator::convertDiscard(const ASTNode& d) {
@@ -752,7 +748,7 @@
"discard statement is only permitted in fragment shaders");
return nullptr;
}
- return std::make_unique<DiscardStatement>(d.fOffset);
+ return DiscardStatement::Make(d.fOffset);
}
std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<Block> main) {
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index d30cbca..0d42f03 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -472,11 +472,11 @@
// This function doesn't return a value, but has early returns, so we've wrapped
// it in a for loop. Use a continue to jump to the end of the loop and "leave"
// the function.
- return std::make_unique<ContinueStatement>(offset);
+ return ContinueStatement::Make(offset);
} else {
// This function doesn't exit early or return a value. A return statement at the
// end is a no-op and can be treated as such.
- return std::make_unique<Nop>();
+ return Nop::Make();
}
}
@@ -486,7 +486,7 @@
SkASSERT(resultExpr);
if (returnComplexity <= ReturnComplexity::kSingleSafeReturn) {
*resultExpr = expr(r.expression());
- return std::make_unique<Nop>();
+ return Nop::Make();
}
// For more complex functions, assign their result into a variable.
@@ -505,7 +505,7 @@
StatementArray block;
block.reserve_back(2);
block.push_back(std::move(assignment));
- block.push_back(std::make_unique<ContinueStatement>(offset));
+ block.push_back(ContinueStatement::Make(offset));
return std::make_unique<Block>(offset, std::move(block), /*symbols=*/nullptr,
/*isScope=*/true);
}
@@ -635,7 +635,7 @@
arguments.size() + // Function arguments (copy out-params back)
1); // Block for inlined code
- inlinedBody.children().push_back(std::make_unique<InlineMarker>(&call->function()));
+ inlinedBody.children().push_back(InlineMarker::Make(&call->function()));
std::unique_ptr<Expression> resultExpr;
if (returnComplexity > ReturnComplexity::kSingleSafeReturn &&
diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp
index ce02ed3..c3bd292 100644
--- a/src/sksl/SkSLRehydrator.cpp
+++ b/src/sksl/SkSLRehydrator.cpp
@@ -355,11 +355,11 @@
isScope);
}
case Rehydrator::kBreak_Command:
- return std::make_unique<BreakStatement>(/*offset=*/-1);
+ return BreakStatement::Make(/*offset=*/-1);
case Rehydrator::kContinue_Command:
- return std::make_unique<ContinueStatement>(/*offset=*/-1);
+ return ContinueStatement::Make(/*offset=*/-1);
case Rehydrator::kDiscard_Command:
- return std::make_unique<DiscardStatement>(/*offset=*/-1);
+ return DiscardStatement::Make(/*offset=*/-1);
case Rehydrator::kDo_Command: {
std::unique_ptr<Statement> stmt = this->statement();
std::unique_ptr<Expression> expr = this->expression();
@@ -390,15 +390,11 @@
case Rehydrator::kInlineMarker_Command: {
const FunctionDeclaration* funcDecl = this->symbolRef<FunctionDeclaration>(
Symbol::Kind::kFunctionDeclaration);
- return std::make_unique<InlineMarker>(funcDecl);
+ return InlineMarker::Make(funcDecl);
}
case Rehydrator::kReturn_Command: {
std::unique_ptr<Expression> expr = this->expression();
- if (expr) {
- return std::make_unique<ReturnStatement>(std::move(expr));
- } else {
- return std::make_unique<ReturnStatement>(/*offset=*/-1);
- }
+ return ReturnStatement::Make(/*offset=*/-1, std::move(expr));
}
case Rehydrator::kSwitch_Command: {
bool isStatic = this->readU8();
diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp
index 2c2e09f..8737068 100644
--- a/src/sksl/dsl/DSLCore.cpp
+++ b/src/sksl/dsl/DSLCore.cpp
@@ -66,11 +66,11 @@
}
static DSLStatement Break() {
- return std::unique_ptr<SkSL::Statement>(new SkSL::BreakStatement(/*offset=*/-1));
+ return SkSL::BreakStatement::Make(/*offset=*/-1);
}
static DSLStatement Continue() {
- return std::unique_ptr<SkSL::Statement>(new SkSL::ContinueStatement(/*offset=*/-1));
+ return SkSL::ContinueStatement::Make(/*offset=*/-1);
}
static DSLStatement Declare(DSLVar& var, PositionInfo pos) {
@@ -82,7 +82,7 @@
}
static DSLStatement Discard() {
- return std::unique_ptr<SkSL::Statement>(new SkSL::DiscardStatement(/*offset=*/-1));
+ return SkSL::DiscardStatement::Make(/*offset=*/-1);
}
static DSLPossibleStatement Do(DSLStatement stmt, DSLExpression test) {
@@ -102,16 +102,11 @@
}
static DSLPossibleStatement Return(DSLExpression value, PositionInfo pos) {
- // note that because Return is called before the function in which it resides exists, at
+ // Note that because Return is called before the function in which it resides exists, at
// this point we do not know the function's return type. We therefore do not check for
// errors, or coerce the value to the correct type, until the return statement is actually
- // added to a function
- std::unique_ptr<SkSL::Expression> expr = value.release();
- if (expr) {
- return std::unique_ptr<SkSL::Statement>(new ReturnStatement(std::move(expr)));
- } else {
- return std::unique_ptr<SkSL::Statement>(new ReturnStatement(/*offset=*/-1));
- }
+ // added to a function. (This is done in IRGenerator::finalizeFunction.)
+ return SkSL::ReturnStatement::Make(/*offset=*/-1, value.release());
}
static DSLExpression Swizzle(DSLExpression base, SkSL::SwizzleComponent::Type a,
diff --git a/src/sksl/ir/SkSLBreakStatement.h b/src/sksl/ir/SkSLBreakStatement.h
index 7a7f39b..34b03fd 100644
--- a/src/sksl/ir/SkSLBreakStatement.h
+++ b/src/sksl/ir/SkSLBreakStatement.h
@@ -23,8 +23,12 @@
BreakStatement(int offset)
: INHERITED(offset, kStatementKind) {}
+ static std::unique_ptr<Statement> Make(int offset) {
+ return std::make_unique<BreakStatement>(offset);
+ }
+
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new BreakStatement(fOffset));
+ return std::make_unique<BreakStatement>(fOffset);
}
String description() const override {
diff --git a/src/sksl/ir/SkSLContinueStatement.h b/src/sksl/ir/SkSLContinueStatement.h
index 54b41ed..959a95e 100644
--- a/src/sksl/ir/SkSLContinueStatement.h
+++ b/src/sksl/ir/SkSLContinueStatement.h
@@ -23,8 +23,12 @@
ContinueStatement(int offset)
: INHERITED(offset, kStatementKind) {}
+ static std::unique_ptr<Statement> Make(int offset) {
+ return std::make_unique<ContinueStatement>(offset);
+ }
+
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new ContinueStatement(fOffset));
+ return std::make_unique<ContinueStatement>(fOffset);
}
String description() const override {
diff --git a/src/sksl/ir/SkSLDiscardStatement.h b/src/sksl/ir/SkSLDiscardStatement.h
index 738924d..df9a54a 100644
--- a/src/sksl/ir/SkSLDiscardStatement.h
+++ b/src/sksl/ir/SkSLDiscardStatement.h
@@ -23,8 +23,12 @@
DiscardStatement(int offset)
: INHERITED(offset, kStatementKind) {}
+ static std::unique_ptr<Statement> Make(int offset) {
+ return std::make_unique<DiscardStatement>(offset);
+ }
+
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new DiscardStatement(fOffset));
+ return std::make_unique<DiscardStatement>(fOffset);
}
String description() const override {
diff --git a/src/sksl/ir/SkSLExpressionStatement.cpp b/src/sksl/ir/SkSLExpressionStatement.cpp
index 17f5de1..3187209 100644
--- a/src/sksl/ir/SkSLExpressionStatement.cpp
+++ b/src/sksl/ir/SkSLExpressionStatement.cpp
@@ -17,7 +17,7 @@
if (context.fConfig->fSettings.fOptimize) {
// Expression-statements without any side effect can be replaced with a Nop.
if (!expr->hasSideEffects()) {
- return std::make_unique<Nop>();
+ return Nop::Make();
}
}
diff --git a/src/sksl/ir/SkSLIfStatement.cpp b/src/sksl/ir/SkSLIfStatement.cpp
index 45b18a7..ad977ff 100644
--- a/src/sksl/ir/SkSLIfStatement.cpp
+++ b/src/sksl/ir/SkSLIfStatement.cpp
@@ -49,7 +49,7 @@
static std::unique_ptr<Statement> replace_empty_with_nop(std::unique_ptr<Statement> stmt,
bool isEmpty) {
return (stmt && (!isEmpty || stmt->is<Nop>())) ? std::move(stmt)
- : std::make_unique<Nop>();
+ : Nop::Make();
}
std::unique_ptr<Statement> IfStatement::Make(const Context& context, int offset, bool isStatic,
diff --git a/src/sksl/ir/SkSLInlineMarker.h b/src/sksl/ir/SkSLInlineMarker.h
index b3e765f..61e4d3b 100644
--- a/src/sksl/ir/SkSLInlineMarker.h
+++ b/src/sksl/ir/SkSLInlineMarker.h
@@ -23,9 +23,13 @@
static constexpr Kind kStatementKind = Kind::kInlineMarker;
InlineMarker(const FunctionDeclaration* function)
- : INHERITED(-1, kStatementKind)
+ : INHERITED(/*offset=*/-1, kStatementKind)
, fFunction(*function) {}
+ static std::unique_ptr<Statement> Make(const FunctionDeclaration* function) {
+ return std::make_unique<InlineMarker>(function);
+ }
+
const FunctionDeclaration& function() const {
return fFunction;
}
diff --git a/src/sksl/ir/SkSLNop.h b/src/sksl/ir/SkSLNop.h
index a21674e..ebd713f 100644
--- a/src/sksl/ir/SkSLNop.h
+++ b/src/sksl/ir/SkSLNop.h
@@ -21,7 +21,11 @@
static constexpr Kind kStatementKind = Kind::kNop;
Nop()
- : INHERITED(-1, kStatementKind) {}
+ : INHERITED(/*offset=*/-1, kStatementKind) {}
+
+ static std::unique_ptr<Statement> Make() {
+ return std::make_unique<Nop>();
+ }
bool isEmpty() const override {
return true;
@@ -32,7 +36,7 @@
}
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new Nop());
+ return std::make_unique<Nop>();
}
private:
diff --git a/src/sksl/ir/SkSLReturnStatement.h b/src/sksl/ir/SkSLReturnStatement.h
index 7f5af69..dc3767d 100644
--- a/src/sksl/ir/SkSLReturnStatement.h
+++ b/src/sksl/ir/SkSLReturnStatement.h
@@ -20,13 +20,14 @@
public:
static constexpr Kind kStatementKind = Kind::kReturn;
- ReturnStatement(int offset)
- : INHERITED(offset, kStatementKind) {}
-
- ReturnStatement(std::unique_ptr<Expression> expression)
- : INHERITED(expression->fOffset, kStatementKind)
+ ReturnStatement(int offset, std::unique_ptr<Expression> expression)
+ : INHERITED(offset, kStatementKind)
, fExpression(std::move(expression)) {}
+ static std::unique_ptr<Statement> Make(int offset, std::unique_ptr<Expression> expression) {
+ return std::make_unique<ReturnStatement>(offset, std::move(expression));
+ }
+
std::unique_ptr<Expression>& expression() {
return fExpression;
}
@@ -40,10 +41,7 @@
}
std::unique_ptr<Statement> clone() const override {
- if (this->expression()) {
- return std::unique_ptr<Statement>(new ReturnStatement(this->expression()->clone()));
- }
- return std::unique_ptr<Statement>(new ReturnStatement(fOffset));
+ return std::make_unique<ReturnStatement>(fOffset, this->expression()->clone());
}
String description() const override {
diff --git a/src/sksl/ir/SkSLSwitchStatement.cpp b/src/sksl/ir/SkSLSwitchStatement.cpp
index 8aff1bc..e70bee3 100644
--- a/src/sksl/ir/SkSLSwitchStatement.cpp
+++ b/src/sksl/ir/SkSLSwitchStatement.cpp
@@ -277,7 +277,7 @@
if (!defaultCase) {
// No default switch-case exists; the switch had no effect.
// We can eliminate the entire switch!
- return std::make_unique<Nop>();
+ return Nop::Make();
}
// We had a default case; that's what we matched with.
matchingCase = defaultCase;