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>

Author: johnstiles-google

src/sksl/SkSLIRGenerator.cpp

@@ -2039,7 +2039,7 @@ std::unique_ptr<Statement> IRGenerator::inlineStatement(
     };
     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,32 +2049,32 @@ std::unique_ptr<Statement> IRGenerator::inlineStatement(
             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);
             return std::make_unique<ForStatement>(offset, std::move(initializer), expr(f.fTest),
                                                   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::unique_ptr<Statement> IRGenerator::inlineStatement(
                     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 @@ std::unique_ptr<Statement> IRGenerator::inlineStatement(
             }
         }
         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::unique_ptr<Statement> IRGenerator::inlineStatement(
                                                      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::unique_ptr<Statement> IRGenerator::inlineStatement(
                                                     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::unique_ptr<Statement> IRGenerator::inlineStatement(
                     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:

tests/SkSLFPTest.cpp

@@ -1118,6 +1118,39 @@ half4 _inlineArghalf4loopyhalf41_0 = %s;
          });
 }
 
+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(),