Convert IRGenerator::convertFor to ForStatement::Make.

Since while statements are implemented in terms of a for loop, also
added ForStatement::MakeWhile() which assumes null for the init-stmt and
the next-expr.

We currently don't have any optimizations for for-statements so the
primary benefit is moving code out of IRGenerator.

Change-Id: I4b3fc3482e28b7d28065e85670a6037b511847ff
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375203
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>

gn/sksl.gni

@@ -100,6 +100,7 @@ skia_sksl_sources = [
   "$_src/sksl/ir/SkSLField.h",
   "$_src/sksl/ir/SkSLFieldAccess.h",
   "$_src/sksl/ir/SkSLFloatLiteral.h",
+  "$_src/sksl/ir/SkSLForStatement.cpp",
   "$_src/sksl/ir/SkSLForStatement.h",
   "$_src/sksl/ir/SkSLFunctionCall.h",
   "$_src/sksl/ir/SkSLFunctionDeclaration.h",

src/sksl/SkSLIRGenerator.cpp

@@ -579,30 +579,6 @@ std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
                              std::move(ifTrue), std::move(ifFalse));
 }
 
-std::unique_ptr<Statement> IRGenerator::convertFor(int offset,
-                                                   std::unique_ptr<Statement> initializer,
-                                                   std::unique_ptr<Expression> test,
-                                                   std::unique_ptr<Expression> next,
-                                                   std::unique_ptr<Statement> statement) {
-    if (test) {
-        test = this->coerce(std::move(test), *fContext.fTypes.fBool);
-        if (!test) {
-            return nullptr;
-        }
-    }
-
-    auto forStmt =
-            std::make_unique<ForStatement>(offset, std::move(initializer), std::move(test),
-                                           std::move(next), std::move(statement), fSymbolTable);
-    if (this->strictES2Mode()) {
-        if (!Analysis::ForLoopIsValidForES2(*forStmt, /*outLoopInfo=*/nullptr,
-                                            &this->errorReporter())) {
-            return nullptr;
-        }
-    }
-    return std::move(forStmt);
-}
-
 std::unique_ptr<Statement> IRGenerator::convertFor(const ASTNode& f) {
     SkASSERT(f.fKind == ASTNode::Kind::kFor);
     AutoSymbolTable table(this);
@@ -636,27 +612,8 @@ std::unique_ptr<Statement> IRGenerator::convertFor(const ASTNode& f) {
         return nullptr;
     }
 
-    return this->convertFor(f.fOffset, std::move(initializer), std::move(test), std::move(next),
+    return ForStatement::Make(fContext, f.fOffset, std::move(initializer), std::move(test),
-                            std::move(statement));
+                              std::move(next), std::move(statement), fSymbolTable);
-}
-
-std::unique_ptr<Statement> IRGenerator::convertWhile(int offset, std::unique_ptr<Expression> test,
-                                                     std::unique_ptr<Statement> statement) {
-    if (this->strictES2Mode()) {
-        this->errorReporter().error(offset, "while loops are not supported");
-        return nullptr;
-    }
-
-    test = this->coerce(std::move(test), *fContext.fTypes.fBool);
-    if (!test) {
-        return nullptr;
-    }
-    if (this->detectVarDeclarationWithoutScope(*statement)) {
-        return nullptr;
-    }
-
-    return std::make_unique<ForStatement>(offset, /*initializer=*/nullptr, std::move(test),
-                                          /*next=*/nullptr, std::move(statement), fSymbolTable);
 }
 
 std::unique_ptr<Statement> IRGenerator::convertWhile(const ASTNode& w) {
@@ -670,7 +627,11 @@ std::unique_ptr<Statement> IRGenerator::convertWhile(const ASTNode& w) {
     if (!statement) {
         return nullptr;
     }
-    return this->convertWhile(w.fOffset, std::move(test), std::move(statement));
+    if (this->detectVarDeclarationWithoutScope(*statement)) {
+        return nullptr;
+    }
+    return ForStatement::MakeWhile(fContext, w.fOffset, std::move(test), std::move(statement),
+                                   fSymbolTable);
 }
 
 std::unique_ptr<Statement> IRGenerator::convertDo(std::unique_ptr<Statement> stmt,
@@ -838,11 +799,9 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
             std::make_unique<IntLiteral>(fContext, /*offset=*/-1, /*value=*/0),
             fContext.fTypes.fInt.get());
     auto initializer = std::make_unique<ExpressionStatement>(std::move(assignment));
-    auto loop = std::make_unique<ForStatement>(/*offset=*/-1,
+    auto loop = ForStatement::Make(
-                                               std::move(initializer),
+            fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next),
-                                               std::move(test), std::move(next),
+            std::make_unique<Block>(/*offset=*/-1, std::move(loopBody)), fSymbolTable);
-                                               std::make_unique<Block>(-1, std::move(loopBody)),
-                                               fSymbolTable);
     StatementArray children;
     children.push_back(std::move(loop));
     return std::make_unique<Block>(/*offset=*/-1, std::move(children));

src/sksl/SkSLIRGenerator.h

@@ -203,11 +203,6 @@ private:
     std::unique_ptr<Statement> convertExpressionStatement(const ASTNode& s);
     std::unique_ptr<Expression> convertField(std::unique_ptr<Expression> base,
                                              StringFragment field);
-    std::unique_ptr<Statement> convertFor(int offset,
-                                          std::unique_ptr<Statement> initializer,
-                                          std::unique_ptr<Expression> test,
-                                          std::unique_ptr<Expression> next,
-                                          std::unique_ptr<Statement> statement);
     std::unique_ptr<Statement> convertFor(const ASTNode& f);
     std::unique_ptr<Expression> convertIdentifier(int offset, StringFragment identifier);
     std::unique_ptr<Expression> convertIdentifier(const ASTNode& identifier);
@@ -238,8 +233,6 @@ private:
                                                          std::unique_ptr<Expression> ifFalse);
     std::unique_ptr<Expression> convertTernaryExpression(const ASTNode& expression);
     std::unique_ptr<Statement> convertVarDeclarationStatement(const ASTNode& s);
-    std::unique_ptr<Statement> convertWhile(int offset, std::unique_ptr<Expression> test,
-                                            std::unique_ptr<Statement> statement);
     std::unique_ptr<Statement> convertWhile(const ASTNode& w);
     void convertGlobalVarDeclarations(const ASTNode& decl);
     void convertEnum(const ASTNode& e);

src/sksl/SkSLInliner.cpp

@@ -461,7 +461,7 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
             // 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.initializer());
-            return std::make_unique<ForStatement>(offset, std::move(initializer), expr(f.test()),
+            return ForStatement::Make(*fContext, offset, std::move(initializer), expr(f.test()),
                                       expr(f.next()), stmt(f.statement()),
                                       SymbolTable::WrapIfBuiltin(f.symbols()));
         }
@@ -726,7 +726,7 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
         inlineStatements = &innerBlock->children();
 
         // for (int _1_loop = 0; _1_loop < 1; _1_loop++) {...}
-        inlinedBody.children().push_back(std::make_unique<ForStatement>(/*offset=*/-1,
+        inlinedBody.children().push_back(ForStatement::Make(*fContext, /*offset=*/-1,
                                                             std::move(loopVar.fVarDecl),
                                                             std::move(test),
                                                             std::move(increment),

src/sksl/dsl/DSLCore.cpp

@@ -93,8 +93,9 @@ public:
 
     static DSLStatement For(DSLStatement initializer, DSLExpression test, DSLExpression next,
                             DSLStatement stmt) {
-        return DSLWriter::IRGenerator().convertFor(/*offset=*/-1, initializer.release(),
+        return ForStatement::Make(DSLWriter::Context(), /*offset=*/-1, initializer.release(),
-                                                   test.release(), next.release(), stmt.release());
+                                  test.release(), next.release(), stmt.release(),
+                                  DSLWriter::SymbolTable());
     }
 
     static DSLStatement If(DSLExpression test, DSLStatement ifTrue, DSLStatement ifFalse) {
@@ -146,7 +147,8 @@ public:
     }
 
     static DSLStatement While(DSLExpression test, DSLStatement stmt) {
-        return DSLWriter::IRGenerator().convertWhile(/*offset=*/-1, test.release(), stmt.release());
+        return ForStatement::MakeWhile(DSLWriter::Context(), /*offset=*/-1, test.release(),
+                                       stmt.release(), DSLWriter::SymbolTable());
     }
 };
 

src/sksl/ir/SkSLForStatement.cpp

@@ -0,0 +1,82 @@
+/*
+ * 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/SkSLAnalysis.h"
+#include "src/sksl/SkSLContext.h"
+#include "src/sksl/SkSLProgramSettings.h"
+#include "src/sksl/ir/SkSLForStatement.h"
+#include "src/sksl/ir/SkSLSymbolTable.h"
+#include "src/sksl/ir/SkSLType.h"
+
+namespace SkSL {
+
+std::unique_ptr<Statement> ForStatement::clone() const {
+    return std::make_unique<ForStatement>(
+            fOffset,
+            this->initializer() ? this->initializer()->clone() : nullptr,
+            this->test() ? this->test()->clone() : nullptr,
+            this->next() ? this->next()->clone() : nullptr,
+            this->statement()->clone(),
+            SymbolTable::WrapIfBuiltin(this->symbols()));
+}
+
+String ForStatement::description() const {
+    String result("for (");
+    if (this->initializer()) {
+        result += this->initializer()->description();
+    } else {
+        result += ";";
+    }
+    result += " ";
+    if (this->test()) {
+        result += this->test()->description();
+    }
+    result += "; ";
+    if (this->next()) {
+        result += this->next()->description();
+    }
+    result += ") " + this->statement()->description();
+    return result;
+}
+
+std::unique_ptr<Statement> ForStatement::Make(const Context& context, int offset,
+                                              std::unique_ptr<Statement> initializer,
+                                              std::unique_ptr<Expression> test,
+                                              std::unique_ptr<Expression> next,
+                                              std::unique_ptr<Statement> statement,
+                                              std::shared_ptr<SymbolTable> symbolTable) {
+    if (test) {
+        test = context.fTypes.fBool->coerceExpression(std::move(test), context);
+        if (!test) {
+            return nullptr;
+        }
+    }
+
+    auto forStmt = std::make_unique<ForStatement>(offset, std::move(initializer), std::move(test),
+                                                  std::move(next), std::move(statement),
+                                                  std::move(symbolTable));
+    if (context.fConfig->strictES2Mode()) {
+        if (!Analysis::ForLoopIsValidForES2(*forStmt, /*outLoopInfo=*/nullptr, &context.fErrors)) {
+            return nullptr;
+        }
+    }
+    return std::move(forStmt);
+}
+
+std::unique_ptr<Statement> ForStatement::MakeWhile(const Context& context, int offset,
+                                                   std::unique_ptr<Expression> test,
+                                                   std::unique_ptr<Statement> statement,
+                                                   std::shared_ptr<SymbolTable> symbolTable) {
+    if (context.fConfig->strictES2Mode()) {
+        context.fErrors.error(offset, "while loops are not supported");
+        return nullptr;
+    }
+    return Make(context, offset, /*initializer=*/nullptr, std::move(test), /*next=*/nullptr,
+                std::move(statement), std::move(symbolTable));
+}
+
+}  // namespace SkSL

src/sksl/ir/SkSLForStatement.h

@@ -31,6 +31,20 @@ public:
     , fNext(std::move(next))
     , fStatement(std::move(statement)) {}
 
+    // Creates an SkSL for loop.
+    static std::unique_ptr<Statement> Make(const Context& context, int offset,
+                                           std::unique_ptr<Statement> initializer,
+                                           std::unique_ptr<Expression> test,
+                                           std::unique_ptr<Expression> next,
+                                           std::unique_ptr<Statement> statement,
+                                           std::shared_ptr<SymbolTable> symbolTable);
+
+    // A while statement is represented in SkSL as a for loop with no init-stmt or next-expr.
+    static std::unique_ptr<Statement> MakeWhile(const Context& context, int offset,
+                                                std::unique_ptr<Expression> test,
+                                                std::unique_ptr<Statement> statement,
+                                                std::shared_ptr<SymbolTable> symbolTable);
+
     std::unique_ptr<Statement>& initializer() {
         return fInitializer;
     }
@@ -67,34 +81,9 @@ public:
         return fSymbolTable;
     }
 
-    std::unique_ptr<Statement> clone() const override {
+    std::unique_ptr<Statement> clone() const override;
-        return std::make_unique<ForStatement>(
-                fOffset,
-                this->initializer() ? this->initializer()->clone() : nullptr,
-                this->test() ? this->test()->clone() : nullptr,
-                this->next() ? this->next()->clone() : nullptr,
-                this->statement()->clone(),
-                SymbolTable::WrapIfBuiltin(this->symbols()));
-    }
 
-    String description() const override {
+    String description() const override;
-        String result("for (");
-        if (this->initializer()) {
-            result += this->initializer()->description();
-        } else {
-            result += ";";
-        }
-        result += " ";
-        if (this->test()) {
-            result += this->test()->description();
-        }
-        result += "; ";
-        if (this->next()) {
-            result += this->next()->description();
-        }
-        result += ") " + this->statement()->description();
-        return result;
-    }
 
 private:
     std::shared_ptr<SymbolTable> fSymbolTable;