Use a scoped block for inlined return statements.
Inlined return statements emit two statements--an assignment, then a
break. If scope braces are otherwise missing, the break statement will
end up in the wrong place. i.e.:
Mistranslated:
if (foo) return bar; --> if (foo) out = bar; break;
Translated properly:
if (foo) return bar; --> if (foo) { out = bar; break; }
Change-Id: Id992abf64ad09e6752f64c71cb556f05c868a453
Bug: skia:10607
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310177
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index a010c56..fbe8559 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -2039,7 +2039,7 @@
};
switch (statement.fKind) {
case Statement::kBlock_Kind: {
- const Block& b = (const Block&) statement;
+ const Block& b = static_cast<const Block&>(statement);
return std::make_unique<Block>(offset, stmts(b.fStatements), b.fSymbols, b.fIsScope);
}
@@ -2049,15 +2049,15 @@
return statement.clone();
case Statement::kDo_Kind: {
- const DoStatement& d = (const DoStatement&) statement;
+ const DoStatement& d = static_cast<const DoStatement&>(statement);
return std::make_unique<DoStatement>(offset, stmt(d.fStatement), expr(d.fTest));
}
case Statement::kExpression_Kind: {
- const ExpressionStatement& e = (const ExpressionStatement&) statement;
+ const ExpressionStatement& e = static_cast<const ExpressionStatement&>(statement);
return std::make_unique<ExpressionStatement>(expr(e.fExpression));
}
case Statement::kFor_Kind: {
- const ForStatement& f = (const ForStatement&) statement;
+ const ForStatement& f = static_cast<const ForStatement&>(statement);
// need to ensure initializer is evaluated first so that we've already remapped its
// declarations by the time we evaluate test & next
std::unique_ptr<Statement> initializer = stmt(f.fInitializer);
@@ -2065,16 +2065,16 @@
expr(f.fNext), stmt(f.fStatement), f.fSymbols);
}
case Statement::kIf_Kind: {
- const IfStatement& i = (const IfStatement&) statement;
+ const IfStatement& i = static_cast<const IfStatement&>(statement);
return std::make_unique<IfStatement>(offset, i.fIsStatic, expr(i.fTest),
stmt(i.fIfTrue), stmt(i.fIfFalse));
}
case Statement::kNop_Kind:
return statement.clone();
case Statement::kReturn_Kind: {
- const ReturnStatement& r = (const ReturnStatement&) statement;
+ const ReturnStatement& r = static_cast<const ReturnStatement&>(statement);
if (r.fExpression) {
- std::unique_ptr<Statement> assignment = std::make_unique<ExpressionStatement>(
+ auto assignment = std::make_unique<ExpressionStatement>(
std::make_unique<BinaryExpression>(
offset,
std::make_unique<VariableReference>(offset, *returnVar,
@@ -2086,9 +2086,10 @@
std::vector<std::unique_ptr<Statement>> block;
block.push_back(std::move(assignment));
block.emplace_back(new BreakStatement(offset));
- return std::make_unique<Block>(offset, std::move(block), nullptr, false);
+ return std::make_unique<Block>(offset, std::move(block), /*symbols=*/nullptr,
+ /*isScope=*/true);
} else {
- return assignment;
+ return std::move(assignment);
}
} else {
if (haveEarlyReturns) {
@@ -2099,7 +2100,7 @@
}
}
case Statement::kSwitch_Kind: {
- const SwitchStatement& ss = (const SwitchStatement&) statement;
+ const SwitchStatement& ss = static_cast<const SwitchStatement&>(statement);
std::vector<std::unique_ptr<SwitchCase>> cases;
for (const auto& sc : ss.fCases) {
cases.emplace_back(new SwitchCase(offset, expr(sc->fValue),
@@ -2109,7 +2110,7 @@
std::move(cases), ss.fSymbols);
}
case Statement::kVarDeclaration_Kind: {
- const VarDeclaration& decl = (const VarDeclaration&) statement;
+ const VarDeclaration& decl = static_cast<const VarDeclaration&>(statement);
std::vector<std::unique_ptr<Expression>> sizes;
for (const auto& size : decl.fSizes) {
sizes.push_back(expr(size));
@@ -2133,7 +2134,8 @@
std::move(initialValue));
}
case Statement::kVarDeclarations_Kind: {
- const VarDeclarations& decls = *((VarDeclarationsStatement&) statement).fDeclaration;
+ const VarDeclarations& decls =
+ *static_cast<const VarDeclarationsStatement&>(statement).fDeclaration;
std::vector<std::unique_ptr<VarDeclaration>> vars;
for (const auto& var : decls.fVars) {
vars.emplace_back((VarDeclaration*) stmt(var).release());
@@ -2143,7 +2145,7 @@
std::make_unique<VarDeclarations>(offset, typePtr, std::move(vars))));
}
case Statement::kWhile_Kind: {
- const WhileStatement& w = (const WhileStatement&) statement;
+ const WhileStatement& w = static_cast<const WhileStatement&>(statement);
return std::make_unique<WhileStatement>(offset, expr(w.fTest), stmt(w.fStatement));
}
default:
diff --git a/tests/SkSLFPTest.cpp b/tests/SkSLFPTest.cpp
index aee05d0..310a57e 100644
--- a/tests/SkSLFPTest.cpp
+++ b/tests/SkSLFPTest.cpp
@@ -1118,6 +1118,39 @@
});
}
+DEF_TEST(SkSLFPIfStatementWithReturnInsideCanBeInlined, r) {
+ test(r,
+ *SkSL::ShaderCapsFactory::Default(),
+ R"__SkSL__(
+ half4 branchy(half4 c) {
+ if (c.z == c.w) return c.yyyy; else return c.zzzz;
+ }
+ void main() {
+ sk_OutColor = branchy(sk_InColor);
+ }
+ )__SkSL__",
+ /*expectedH=*/{},
+ /*expectedCPP=*/{
+ R"__Cpp__(fragBuilder->codeAppendf(
+R"SkSL(half4 _inlineResulthalf4branchyhalf40;
+half4 _inlineArghalf4branchyhalf41_0 = %s;
+do {
+ if (_inlineArghalf4branchyhalf41_0.z == _inlineArghalf4branchyhalf41_0.w) {
+ _inlineResulthalf4branchyhalf40 = _inlineArghalf4branchyhalf41_0.yyyy;
+ break;
+ } else {
+ _inlineResulthalf4branchyhalf40 = _inlineArghalf4branchyhalf41_0.zzzz;
+ break;
+ }
+} while (false);
+%s = _inlineResulthalf4branchyhalf40;
+
+)SkSL"
+, args.fInputColor, args.fOutputColor);
+)__Cpp__",
+ });
+}
+
DEF_TEST(SkSLFPMatrixSampleConstant, r) {
test(r,
*SkSL::ShaderCapsFactory::Default(),