SkSL For statement now enforces correct initializer type

Bug: skia:11870
Change-Id: I960800ac554e3c189d0b708fa6cabccf071fd020
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397460
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>

src/sksl/dsl/DSLCore.cpp

@@ -88,7 +88,7 @@ public:
     }
 
     static DSLPossibleStatement For(DSLStatement initializer, DSLExpression test,
-                                    DSLExpression next, DSLStatement stmt) {
+                                    DSLExpression next, DSLStatement stmt, PositionInfo pos) {
         return ForStatement::Convert(DSLWriter::Context(), /*offset=*/-1, initializer.release(),
                                      test.release(), next.release(), stmt.release(),
                                      DSLWriter::SymbolTable());
@@ -192,7 +192,7 @@ DSLStatement Do(DSLStatement stmt, DSLExpression test, PositionInfo pos) {
 DSLStatement For(DSLStatement initializer, DSLExpression test, DSLExpression next,
                  DSLStatement stmt, PositionInfo pos) {
     return DSLStatement(DSLCore::For(std::move(initializer), std::move(test), std::move(next),
-                                     std::move(stmt)), pos);
+                                     std::move(stmt), pos), pos);
 }
 
 DSLStatement If(DSLExpression test, DSLStatement ifTrue, DSLStatement ifFalse, PositionInfo pos) {

src/sksl/ir/SkSLForStatement.cpp

@@ -8,9 +8,12 @@
 #include "src/sksl/SkSLAnalysis.h"
 #include "src/sksl/SkSLContext.h"
 #include "src/sksl/SkSLProgramSettings.h"
+#include "src/sksl/ir/SkSLBlock.h"
+#include "src/sksl/ir/SkSLExpressionStatement.h"
 #include "src/sksl/ir/SkSLForStatement.h"
 #include "src/sksl/ir/SkSLSymbolTable.h"
 #include "src/sksl/ir/SkSLType.h"
+#include "src/sksl/ir/SkSLVarDeclarations.h"
 
 namespace SkSL {
 
@@ -49,6 +52,10 @@ std::unique_ptr<Statement> ForStatement::Convert(const Context& context, int off
                                                  std::unique_ptr<Expression> next,
                                                  std::unique_ptr<Statement> statement,
                                                  std::shared_ptr<SymbolTable> symbolTable) {
+    if (!IsValidInitializer(initializer.get())) {
+        context.fErrors.error(initializer->fOffset, "invalid for loop initializer");
+        return nullptr;
+    }
     if (test) {
         test = context.fTypes.fBool->coerceExpression(std::move(test), context);
         if (!test) {
@@ -85,6 +92,7 @@ std::unique_ptr<Statement> ForStatement::Make(const Context& context, int offset
                                               std::unique_ptr<Expression> next,
                                               std::unique_ptr<Statement> statement,
                                               std::shared_ptr<SymbolTable> symbolTable) {
+    SkASSERT(IsValidInitializer(initializer.get()));
     SkASSERT(!test || test->type() == *context.fTypes.fBool);
     SkASSERT(!context.fConfig->strictES2Mode() ||
              Analysis::ForLoopIsValidForES2(offset, initializer.get(), test.get(), next.get(),
@@ -96,4 +104,25 @@ std::unique_ptr<Statement> ForStatement::Make(const Context& context, int offset
                                           std::move(symbolTable));
 }
 
+static bool is_vardecl_block(const SkSL::Statement& stmt) {
+    if (!stmt.is<SkSL::Block>()) {
+        return false;
+    }
+    const SkSL::Block& b = stmt.as<SkSL::Block>();
+    if (b.isScope()) {
+        return false;
+    }
+    for (const auto& child : b.children()) {
+        if (!child->is<SkSL::VarDeclaration>()) {
+            return false;
+        }
+    }
+    return true;
+}
+
+bool ForStatement::IsValidInitializer(const Statement* stmt) {
+    return !stmt || stmt->is<SkSL::VarDeclaration>() || stmt->is<SkSL::ExpressionStatement>() ||
+           is_vardecl_block(*stmt);
+}
+
 }  // namespace SkSL

src/sksl/ir/SkSLForStatement.h

@@ -53,6 +53,8 @@ public:
                                            std::unique_ptr<Statement> statement,
                                            std::shared_ptr<SymbolTable> symbolTable);
 
+    static bool IsValidInitializer(const Statement* stmt);
+
     std::unique_ptr<Statement>& initializer() {
         return fInitializer;
     }

tests/SkSLDSLTest.cpp

@@ -1114,6 +1114,11 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFor, r, ctxInfo) {
         ExpectError error(r, "error: expected 'bool', but found 'int'\n");
         For(i = 0, i + 10, ++i, i += 5).release();
     }
+
+    {
+        ExpectError error(r, "error: invalid for loop initializer\n");
+        For(If(i == 0, i = 1), i < 10, ++i, i += 5).release();
+    }
 }
 
 DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLFunction, r, ctxInfo) {