refactored SkSLVarDeclaration out of existence

Bug: skia:
Change-Id: I3dbc08e6d759f6828a472246d4797babb6cc132e
Reviewed-on: https://skia-review.googlesource.com/66147
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

16 files changed, 201 insertions, 220 deletions

diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index 5fd4229457..a0282a4aca 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -123,6 +123,46 @@ bool BasicBlock::tryRemoveExpressionBefore(std::vector<BasicBlock::Node>::iterat
     return result;
 }
 
+bool BasicBlock::tryRemoveExpressionAfter(std::vector<BasicBlock::Node>::iterator* iter,
+                                          Expression* e) {
+    if (e->fKind == Expression::kTernary_Kind) {
+        return false;
+    }
+    bool result;
+    if ((*iter)->fKind == BasicBlock::Node::kExpression_Kind) {
+        ASSERT((*iter)->expression()->get() != e);
+        Expression* old = (*iter)->expression()->get();
+        do {
+            if ((*iter) == fNodes.end()) {
+                return false;
+            }
+            ++(*iter);
+        } while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
+                 (*iter)->expression()->get() != e);
+        result = this->tryRemoveExpression(iter);
+        while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
+               (*iter)->expression()->get() != old) {
+            ASSERT(*iter != fNodes.begin());
+            --(*iter);
+        }
+    } else {
+        Statement* old = (*iter)->statement()->get();
+        do {
+            if ((*iter) == fNodes.end()) {
+                return false;
+            }
+            ++(*iter);
+        } while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
+                 (*iter)->expression()->get() != e);
+        result = this->tryRemoveExpression(iter);
+        while ((*iter)->fKind != BasicBlock::Node::kStatement_Kind ||
+               (*iter)->statement()->get() != old) {
+            ASSERT(*iter != fNodes.begin());
+            --(*iter);
+        }
+    }
+    return result;
+}
 bool BasicBlock::tryRemoveLValueBefore(std::vector<BasicBlock::Node>::iterator* iter,
                                        Expression* lvalue) {
     switch (lvalue->fKind) {
@@ -466,20 +506,14 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
             break;
         }
         case Statement::kVarDeclarations_Kind: {
+            cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
+                                                         nullptr, s });
             VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) **s);
-            for (auto& stmt : decls.fDeclaration->fVars) {
-                if (stmt->fKind == Statement::kNop_Kind) {
-                    continue;
-                }
-                VarDeclaration& vd = (VarDeclaration&) *stmt;
-                if (vd.fValue) {
-                    this->addExpression(cfg, &vd.fValue, true);
+            for (Variable* var : decls.fDeclaration->fVars) {
+                if (var->fInitialValue) {
+                    this->addExpression(cfg, &var->fInitialValue, true);
                 }
-                cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind,
-                                                             false, nullptr, &stmt });
             }
-            cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
-                                                         nullptr, s });
             break;
         }
         case Statement::kDiscard_Kind:
diff --git a/src/sksl/SkSLCFGGenerator.h b/src/sksl/SkSLCFGGenerator.h
index 885d9261ec..3fe0d2a47e 100644
--- a/src/sksl/SkSLCFGGenerator.h
+++ b/src/sksl/SkSLCFGGenerator.h
@@ -96,6 +96,14 @@ struct BasicBlock {
     bool tryRemoveExpressionBefore(std::vector<BasicBlock::Node>::iterator* iter, Expression* e);
 
     /**
+     * Locates and attempts remove an expression occurring after the expression pointed to by iter.
+     * If the expression can be cleanly removed, returns true and resets iter to a valid iterator
+     * pointing to the same expression it did initially. Otherwise returns false (and the CFG will
+     * need to be regenerated).
+     */
+    bool tryRemoveExpressionAfter(std::vector<BasicBlock::Node>::iterator* iter, Expression* e);
+
+    /**
      * As tryRemoveExpressionBefore, but for lvalues. As lvalues are at most partially evaluated
      * (for instance, x[i] = 0 evaluates i but not x) this will only look for the parts of the
      * lvalue that are actually evaluated.
diff --git a/src/sksl/SkSLCPPCodeGenerator.cpp b/src/sksl/SkSLCPPCodeGenerator.cpp
index 506daa8b4f..184453812a 100644
--- a/src/sksl/SkSLCPPCodeGenerator.cpp
+++ b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -273,11 +273,10 @@ void CPPCodeGenerator::writeFunctionCall(const FunctionCall& c) {
         for (const auto& p : fProgram.fElements) {
             if (ProgramElement::kVar_Kind == p->fKind) {
                 const VarDeclarations* decls = (const VarDeclarations*) p.get();
-                for (const auto& raw : decls->fVars) {
-                    VarDeclaration& decl = (VarDeclaration&) *raw;
-                    if (decl.fVar == &((VariableReference&) *c.fArguments[0]).fVariable) {
+                for (const auto& var : decls->fVars) {
+                    if (var == &((VariableReference&) *c.fArguments[0]).fVariable) {
                         found = true;
-                    } else if (decl.fVar->fType == *fContext.fFragmentProcessor_Type) {
+                    } else if (var->fType == *fContext.fFragmentProcessor_Type) {
                         ++index;
                     }
                 }
@@ -353,7 +352,7 @@ void CPPCodeGenerator::writeProgramElement(const ProgramElement& p) {
         if (!decls.fVars.size()) {
             return;
         }
-        const Variable& var = *((VarDeclaration&) *decls.fVars[0]).fVar;
+        const Variable& var = *decls.fVars[0];
         if (var.fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kUniform_Flag) ||
             -1 != var.fModifiers.fLayout.fBuiltin) {
             return;
@@ -413,18 +412,17 @@ void CPPCodeGenerator::writePrivateVars() {
     for (const auto& p : fProgram.fElements) {
         if (ProgramElement::kVar_Kind == p->fKind) {
             const VarDeclarations* decls = (const VarDeclarations*) p.get();
-            for (const auto& raw : decls->fVars) {
-                VarDeclaration& decl = (VarDeclaration&) *raw;
-                if (is_private(*decl.fVar)) {
-                    if (decl.fVar->fType == *fContext.fFragmentProcessor_Type) {
-                        fErrors.error(decl.fOffset,
+            for (const auto& var : decls->fVars) {
+                if (is_private(*var)) {
+                    if (var->fType == *fContext.fFragmentProcessor_Type) {
+                        fErrors.error(var->fOffset,
                                       "fragmentProcessor variables must be declared 'in'");
                         return;
                     }
                     this->writef("%s %s;\n",
-                                 HCodeGenerator::FieldType(fContext, decl.fVar->fType,
-                                                           decl.fVar->fModifiers.fLayout).c_str(),
-                                 String(decl.fVar->fName).c_str());
+                                 HCodeGenerator::FieldType(fContext, var->fType,
+                                                           var->fModifiers.fLayout).c_str(),
+                                 String(var->fName).c_str());
                 }
             }
         }
@@ -435,12 +433,11 @@ void CPPCodeGenerator::writePrivateVarValues() {
     for (const auto& p : fProgram.fElements) {
         if (ProgramElement::kVar_Kind == p->fKind) {
             const VarDeclarations* decls = (const VarDeclarations*) p.get();
-            for (const auto& raw : decls->fVars) {
-                VarDeclaration& decl = (VarDeclaration&) *raw;
-                if (is_private(*decl.fVar) && decl.fValue) {
-                    this->writef("%s = ", String(decl.fVar->fName).c_str());
+            for (const auto& var : decls->fVars) {
+                if (is_private(*var) && var->fInitialValue) {
+                    this->writef("%s = ", String(var->fName).c_str());
                     fCPPMode = true;
-                    this->writeExpression(*decl.fValue, kAssignment_Precedence);
+                    this->writeExpression(*var->fInitialValue, kAssignment_Precedence);
                     fCPPMode = false;
                     this->write(";\n");
                 }
@@ -501,12 +498,11 @@ bool CPPCodeGenerator::writeEmitCode(std::vector<const Variable*>& uniforms) {
     for (const auto& p : fProgram.fElements) {
         if (ProgramElement::kVar_Kind == p->fKind) {
             const VarDeclarations* decls = (const VarDeclarations*) p.get();
-            for (const auto& raw : decls->fVars) {
-                VarDeclaration& decl = (VarDeclaration&) *raw;
-                String nameString(decl.fVar->fName);
+            for (const auto& var : decls->fVars) {
+                String nameString(var->fName);
                 const char* name = nameString.c_str();
-                if (SectionAndParameterHelper::IsParameter(*decl.fVar) &&
-                    is_accessible(*decl.fVar)) {
+                if (SectionAndParameterHelper::IsParameter(*var) &&
+                    is_accessible(*var)) {
                     this->writef("        auto %s = _outer.%s();\n"
                                  "        (void) %s;\n",
                                  name, name, name);
@@ -573,16 +569,15 @@ void CPPCodeGenerator::writeSetData(std::vector<const Variable*>& uniforms) {
         for (const auto& p : fProgram.fElements) {
             if (ProgramElement::kVar_Kind == p->fKind) {
                 const VarDeclarations* decls = (const VarDeclarations*) p.get();
-                for (const auto& raw : decls->fVars) {
-                    VarDeclaration& decl = (VarDeclaration&) *raw;
-                    String nameString(decl.fVar->fName);
+                for (const Variable* var : decls->fVars) {
+                    String nameString(var->fName);
                     const char* name = nameString.c_str();
-                    if (needs_uniform_var(*decl.fVar)) {
+                    if (needs_uniform_var(*var)) {
                         this->writef("        UniformHandle& %s = %sVar;\n"
                                      "        (void) %s;\n",
                                      name, HCodeGenerator::FieldName(name).c_str(), name);
-                    } else if (SectionAndParameterHelper::IsParameter(*decl.fVar) &&
-                               decl.fVar->fType != *fContext.fFragmentProcessor_Type) {
+                    } else if (SectionAndParameterHelper::IsParameter(*var) &&
+                               var->fType != *fContext.fFragmentProcessor_Type) {
                         if (!wroteProcessor) {
                             this->writef("        const %s& _outer = _proc.cast<%s>();\n", fullName,
                                          fullName);
@@ -718,11 +713,10 @@ bool CPPCodeGenerator::generateCode() {
     for (const auto& p : fProgram.fElements) {
         if (ProgramElement::kVar_Kind == p->fKind) {
             const VarDeclarations* decls = (const VarDeclarations*) p.get();
-            for (const auto& raw : decls->fVars) {
-                VarDeclaration& decl = (VarDeclaration&) *raw;
-                if ((decl.fVar->fModifiers.fFlags & Modifiers::kUniform_Flag) &&
-                           decl.fVar->fType.kind() != Type::kSampler_Kind) {
-                    uniforms.push_back(decl.fVar);
+            for (const Variable* var : decls->fVars) {
+                if ((var->fModifiers.fFlags & Modifiers::kUniform_Flag) &&
+                           var->fType.kind() != Type::kSampler_Kind) {
+                    uniforms.push_back(var);
                 }
             }
         }
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 0b51824f0d..1b04bb1dea 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -189,12 +189,14 @@ Compiler::Compiler(Flags flags)
 
     StringFragment skCapsName("sk_Caps");
     Variable* skCaps = new Variable(-1, Modifiers(), skCapsName,
-                                    *fContext.fSkCaps_Type, Variable::kGlobal_Storage);
+                                    *fContext.fSkCaps_Type, Variable::kGlobal_Storage, nullptr,
+                                    {});
     fIRGenerator->fSymbolTable->add(skCapsName, std::unique_ptr<Symbol>(skCaps));
 
     StringFragment skArgsName("sk_Args");
     Variable* skArgs = new Variable(-1, Modifiers(), skArgsName,
-                                    *fContext.fSkArgs_Type, Variable::kGlobal_Storage);
+                                    *fContext.fSkArgs_Type, Variable::kGlobal_Storage, nullptr,
+                                    {});
     fIRGenerator->fSymbolTable->add(skArgsName, std::unique_ptr<Symbol>(skArgs));
 
     std::vector<std::unique_ptr<ProgramElement>> ignored;
@@ -307,10 +309,11 @@ void Compiler::addDefinitions(const BasicBlock::Node& node,
         }
         case BasicBlock::Node::kStatement_Kind: {
             const Statement* stmt = (Statement*) node.statement()->get();
-            if (stmt->fKind == Statement::kVarDeclaration_Kind) {
-                VarDeclaration& vd = (VarDeclaration&) *stmt;
-                if (vd.fValue) {
-                    (*definitions)[vd.fVar] = &vd.fValue;
+            if (stmt->fKind == Statement::kVarDeclarations_Kind) {
+                for (Variable* var : ((VarDeclarationsStatement*) stmt)->fDeclaration->fVars) {
+                    if (var->fInitialValue) {
+                        (*definitions)[var] = &var->fInitialValue;
+                    }
                 }
             }
             break;
@@ -366,10 +369,8 @@ static DefinitionMap compute_start_state(const CFG& cfg) {
                 const Statement* s = node.statement()->get();
                 if (s->fKind == Statement::kVarDeclarations_Kind) {
                     const VarDeclarationsStatement* vd = (const VarDeclarationsStatement*) s;
-                    for (const auto& decl : vd->fDeclaration->fVars) {
-                        if (decl->fKind == Statement::kVarDeclaration_Kind) {
-                            result[((VarDeclaration&) *decl).fVar] = nullptr;
-                        }
+                    for (const Variable* var : vd->fDeclaration->fVars) {
+                        result[var] = nullptr;
                     }
                 }
             }
@@ -644,8 +645,7 @@ void Compiler::simplifyExpression(DefinitionMap& definitions,
             if (var.fStorage == Variable::kLocal_Storage && !definitions[&var] &&
                 (*undefinedVariables).find(&var) == (*undefinedVariables).end()) {
                 (*undefinedVariables).insert(&var);
-                this->error(expr->fOffset,
-                            "'" + var.fName + "' has not been assigned");
+                this->error(expr->fOffset, "'" + var.fName + "' has not been assigned");
             }
             break;
         }
@@ -890,19 +890,25 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
                                  bool* outNeedsRescan) {
     Statement* stmt = (*iter)->statement()->get();
     switch (stmt->fKind) {
-        case Statement::kVarDeclaration_Kind: {
-            const auto& varDecl = (VarDeclaration&) *stmt;
-            if (varDecl.fVar->dead() &&
-                (!varDecl.fValue ||
-                 !varDecl.fValue->hasSideEffects())) {
-                if (varDecl.fValue) {
-                    ASSERT((*iter)->statement()->get() == stmt);
-                    if (!b.tryRemoveExpressionBefore(iter, varDecl.fValue.get())) {
-                        *outNeedsRescan = true;
+        case Statement::kVarDeclarations_Kind: {
+            std::vector<Variable*>& vars = ((VarDeclarationsStatement*) stmt)->fDeclaration->fVars;
+            for (auto varIter = vars.begin(); varIter != vars.end();) {
+                Variable* var = *varIter;
+                if (var->dead() && (!var->fInitialValue || !var->fInitialValue->hasSideEffects())) {
+                    if (var->fInitialValue) {
+                        ASSERT((*iter)->statement()->get() == stmt);
+                        if (!b.tryRemoveExpressionAfter(iter, var->fInitialValue.get())) {
+                            *outNeedsRescan = true;
+                        }
                     }
+                    varIter = vars.erase(varIter);
+                    *outUpdated = true;
+                } else {
+                    ++varIter;
                 }
+            }
+            if (!vars.size()) {
                 (*iter)->setStatement(std::unique_ptr<Statement>(new Nop()));
-                *outUpdated = true;
             }
             break;
         }
@@ -1094,22 +1100,6 @@ void Compiler::scanCFG(FunctionDefinition& f) {
                         }
                         ++iter;
                         break;
-                    case Statement::kVarDeclarations_Kind: {
-                        VarDeclarations& decls = *((VarDeclarationsStatement&) s).fDeclaration;
-                        for (auto varIter = decls.fVars.begin(); varIter != decls.fVars.end();) {
-                            if ((*varIter)->fKind == Statement::kNop_Kind) {
-                                varIter = decls.fVars.erase(varIter);
-                            } else {
-                                ++varIter;
-                            }
-                        }
-                        if (!decls.fVars.size()) {
-                            iter = b.fNodes.erase(iter);
-                        } else {
-                            ++iter;
-                        }
-                        break;
-                    }
                     default:
                         ++iter;
                         break;
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 22331cdc80..e3c2003a3f 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -789,30 +789,29 @@ void GLSLCodeGenerator::writeTypePrecision(const Type& type) {
 void GLSLCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool global) {
     ASSERT(decl.fVars.size() > 0);
     bool wroteType = false;
-    for (const auto& stmt : decl.fVars) {
-        VarDeclaration& var = (VarDeclaration&) *stmt;
+    for (const Variable* var : decl.fVars) {
         if (wroteType) {
             this->write(", ");
         } else {
-            this->writeModifiers(var.fVar->fModifiers, global);
+            this->writeModifiers(var->fModifiers, global);
             this->writeTypePrecision(decl.fBaseType);
             this->writeType(decl.fBaseType);
             this->write(" ");
             wroteType = true;
         }
-        this->write(var.fVar->fName);
-        for (const auto& size : var.fSizes) {
+        this->write(var->fName);
+        for (const auto& size : var->fSizes) {
             this->write("[");
             if (size) {
                 this->writeExpression(*size, kTopLevel_Precedence);
             }
             this->write("]");
         }
-        if (var.fValue) {
+        if (var->fInitialValue) {
             this->write(" = ");
-            this->writeVarInitializer(*var.fVar, *var.fValue);
+            this->writeVarInitializer(*var, *var->fInitialValue);
         }
-        if (!fFoundImageDecl && var.fVar->fType == *fContext.fImage2D_Type) {
+        if (!fFoundImageDecl && var->fType == *fContext.fImage2D_Type) {
             if (fProgram.fSettings.fCaps->imageLoadStoreExtensionString()) {
                 fHeader.writeText("#extension ");
                 fHeader.writeText(fProgram.fSettings.fCaps->imageLoadStoreExtensionString());
@@ -984,7 +983,7 @@ void GLSLCodeGenerator::writeProgramElement(const ProgramElement& e) {
         case ProgramElement::kVar_Kind: {
             VarDeclarations& decl = (VarDeclarations&) e;
             if (decl.fVars.size() > 0) {
-                int builtin = ((VarDeclaration&) *decl.fVars[0]).fVar->fModifiers.fLayout.fBuiltin;
+                int builtin = decl.fVars[0]->fModifiers.fLayout.fBuiltin;
                 if (builtin == -1) {
                     // normal var
                     this->writeVarDeclarations(decl, true);
diff --git a/src/sksl/SkSLGLSLCodeGenerator.h b/src/sksl/SkSLGLSLCodeGenerator.h
index e9a63ac48f..5e1dffe70a 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.h
+++ b/src/sksl/SkSLGLSLCodeGenerator.h
@@ -116,8 +116,6 @@ protected:
 
     void writeModifiers(const Modifiers& modifiers, bool globalContext);
 
-    void writeGlobalVars(const VarDeclaration& vs);
-
     virtual void writeVarInitializer(const Variable& var, const Expression& value);
 
     const char* getTypePrecision(const Type& type);
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 2fb27fcec4..7bbb6da362 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -219,7 +219,7 @@ std::unique_ptr<Statement> IRGenerator::convertVarDeclarationStatement(
 
 std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVarDeclarations& decl,
                                                                      Variable::Storage storage) {
-    std::vector<std::unique_ptr<VarDeclaration>> variables;
+    std::vector<Variable*> variables;
     const Type* baseType = this->convertType(*decl.fType);
     if (!baseType) {
         return nullptr;
@@ -254,8 +254,6 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa
                 sizes.push_back(nullptr);
             }
         }
-        auto var = std::unique_ptr<Variable>(new Variable(decl.fOffset, decl.fModifiers,
-                                                          varDecl.fName, *type, storage));
         std::unique_ptr<Expression> value;
         if (varDecl.fValue) {
             value = this->convertExpression(*varDecl.fValue);
@@ -263,6 +261,11 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa
                 return nullptr;
             }
             value = this->coerce(std::move(value), *type);
+        }
+        auto var = std::unique_ptr<Variable>(new Variable(decl.fOffset, decl.fModifiers,
+                                                          varDecl.fName, *type, storage,
+                                                          std::move(value), std::move(sizes)));
+        if (varDecl.fValue) {
             var->fWriteCount = 1;
         }
         if (storage == Variable::kGlobal_Storage && varDecl.fName == "sk_FragColor" &&
@@ -275,8 +278,7 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa
             Variable* old = (Variable*) (*fSymbolTable)[varDecl.fName];
             old->fModifiers = var->fModifiers;
         } else {
-            variables.emplace_back(new VarDeclaration(var.get(), std::move(sizes),
-                                                      std::move(value)));
+            variables.push_back(var.get());
             fSymbolTable->add(varDecl.fName, std::move(var));
         }
     }
@@ -531,7 +533,7 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
                                                                           std::move(main))));
     fSymbolTable->add(invokeDecl->fName, std::unique_ptr<FunctionDeclaration>(invokeDecl));
 
-    std::vector<std::unique_ptr<VarDeclaration>> variables;
+    std::vector<Variable*> variables;
     Variable* loopIdx = (Variable*) (*fSymbolTable)["sk_InvocationID"];
     ASSERT(loopIdx);
     std::unique_ptr<Expression> test(new BinaryExpression(-1,
@@ -598,7 +600,7 @@ void IRGenerator::convertFunction(const ASTFunction& f,
         }
         StringFragment name = param->fName;
         Variable* var = new Variable(param->fOffset, param->fModifiers, name, *type,
-                                     Variable::kParameter_Storage);
+                                     Variable::kParameter_Storage, nullptr, {});
         fSymbolTable->takeOwnership(var);
         parameters.push_back(var);
     }
@@ -712,29 +714,27 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte
         if (!decl) {
             return nullptr;
         }
-        for (const auto& stmt : decl->fVars) {
-            VarDeclaration& vd = (VarDeclaration&) *stmt;
+        for (const Variable* var : decl->fVars) {
             if (haveRuntimeArray) {
-                fErrors.error(decl->fOffset,
+                fErrors.error(var->fOffset,
                               "only the last entry in an interface block may be a runtime-sized "
                               "array");
             }
-            fields.push_back(Type::Field(vd.fVar->fModifiers, vd.fVar->fName,
-                                         &vd.fVar->fType));
-            if (vd.fValue) {
+            fields.push_back(Type::Field(var->fModifiers, var->fName, &var->fType));
+            if (var->fInitialValue) {
                 fErrors.error(decl->fOffset,
                               "initializers are not permitted on interface block fields");
             }
-            if (vd.fVar->fModifiers.fFlags & (Modifiers::kIn_Flag |
-                                                Modifiers::kOut_Flag |
-                                                Modifiers::kUniform_Flag |
-                                                Modifiers::kBuffer_Flag |
-                                                Modifiers::kConst_Flag)) {
+            if (var->fModifiers.fFlags & (Modifiers::kIn_Flag |
+                                          Modifiers::kOut_Flag |
+                                          Modifiers::kUniform_Flag |
+                                          Modifiers::kBuffer_Flag |
+                                          Modifiers::kConst_Flag)) {
                 fErrors.error(decl->fOffset,
                               "interface block fields may not have storage qualifiers");
             }
-            if (vd.fVar->fType.kind() == Type::kArray_Kind &&
-                vd.fVar->fType.columns() == -1) {
+            if (var->fType.kind() == Type::kArray_Kind &&
+                var->fType.columns() == -1) {
                 haveRuntimeArray = true;
             }
         }
@@ -771,7 +771,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte
     }
     Variable* var = new Variable(intf.fOffset, intf.fModifiers,
                                  intf.fInstanceName.fLength ? intf.fInstanceName : intf.fTypeName,
-                                 *type, Variable::kGlobal_Storage);
+                                 *type, Variable::kGlobal_Storage, nullptr, {});
     old->takeOwnership(var);
     if (intf.fInstanceName.fLength) {
         old->addWithoutOwnership(intf.fInstanceName, var);
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
index de6b1d0854..713c28a3ed 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -538,34 +538,33 @@ void MetalCodeGenerator::writeVarInitializer(const Variable& var, const Expressi
 void MetalCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool global) {
     ASSERT(decl.fVars.size() > 0);
     bool wroteType = false;
-    for (const auto& stmt : decl.fVars) {
-        VarDeclaration& var = (VarDeclaration&) *stmt;
-        if (var.fVar->fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag |
-                                           Modifiers::kUniform_Flag)) {
+    for (const Variable* var : decl.fVars) {
+        if (var->fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag |
+                                      Modifiers::kUniform_Flag)) {
             ASSERT(global);
             continue;
         }
         if (wroteType) {
             this->write(", ");
         } else {
-            this->writeModifiers(var.fVar->fModifiers, global);
+            this->writeModifiers(var->fModifiers, global);
             this->writeType(decl.fBaseType);
             this->write(" ");
             wroteType = true;
         }
-        this->write(var.fVar->fName);
-        for (const auto& size : var.fSizes) {
+        this->write(var->fName);
+        for (const auto& size : var->fSizes) {
             this->write("[");
             if (size) {
                 this->writeExpression(*size, kTopLevel_Precedence);
             }
             this->write("]");
         }
-        if (var.fValue) {
+        if (var->fInitialValue) {
             this->write(" = ");
-            this->writeVarInitializer(*var.fVar, *var.fValue);
+            this->writeVarInitializer(*var, *var->fInitialValue);
         }
-        if (!fFoundImageDecl && var.fVar->fType == *fContext.fImage2D_Type) {
+        if (!fFoundImageDecl && var->fType == *fContext.fImage2D_Type) {
             if (fProgram.fSettings.fCaps->imageLoadStoreExtensionString()) {
                 fHeader.writeText("#extension ");
                 fHeader.writeText(fProgram.fSettings.fCaps->imageLoadStoreExtensionString());
@@ -733,7 +732,7 @@ void MetalCodeGenerator::writeUniformStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
+            const Variable& first = *decls.fVars[0];
             if (first.fModifiers.fFlags & Modifiers::kUniform_Flag) {
                 if (-1 == fUniformBuffer) {
                     this->write("struct Uniforms {\n");
@@ -750,9 +749,8 @@ void MetalCodeGenerator::writeUniformStruct() {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const auto& stmt : decls.fVars) {
-                    VarDeclaration& var = (VarDeclaration&) *stmt;
-                    this->write(var.fVar->fName);
+                for (const auto& var : decls.fVars) {
+                    this->write(var->fName);
                 }
                 this->write(";\n");
             }
@@ -774,18 +772,17 @@ void MetalCodeGenerator::writeInputStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
+            const Variable& first = *decls.fVars[0];
             if (first.fModifiers.fFlags & Modifiers::kIn_Flag &&
                 -1 == first.fModifiers.fLayout.fBuiltin) {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const auto& stmt : decls.fVars) {
-                    VarDeclaration& var = (VarDeclaration&) *stmt;
-                    this->write(var.fVar->fName);
-                    if (-1 != var.fVar->fModifiers.fLayout.fLocation) {
+                for (const Variable* var : decls.fVars) {
+                    this->write(var->fName);
+                    if (-1 != var->fModifiers.fLayout.fLocation) {
                         this->write("  [[attribute(" +
-                                    to_string(var.fVar->fModifiers.fLayout.fLocation) + ")]]");
+                                    to_string(var->fModifiers.fLayout.fLocation) + ")]]");
                     }
                 }
                 this->write(";\n");
@@ -804,15 +801,14 @@ void MetalCodeGenerator::writeOutputStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
+            const Variable& first = *decls.fVars[0];
             if (first.fModifiers.fFlags & Modifiers::kOut_Flag &&
                 -1 == first.fModifiers.fLayout.fBuiltin) {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const auto& stmt : decls.fVars) {
-                    VarDeclaration& var = (VarDeclaration&) *stmt;
-                    this->write(var.fVar->fName);
+                for (const Variable* var : decls.fVars) {
+                    this->write(var->fName);
                 }
                 this->write(";\n");
             }
@@ -827,7 +823,7 @@ void MetalCodeGenerator::writeProgramElement(const ProgramElement& e) {
         case ProgramElement::kVar_Kind: {
             VarDeclarations& decl = (VarDeclarations&) e;
             if (decl.fVars.size() > 0) {
-                int builtin = ((VarDeclaration&) *decl.fVars[0]).fVar->fModifiers.fLayout.fBuiltin;
+                int builtin = decl.fVars[0]->fModifiers.fLayout.fBuiltin;
                 if (-1 == builtin) {
                     // normal var
                     this->writeVarDeclarations(decl, true);
diff --git a/src/sksl/SkSLMetalCodeGenerator.h b/src/sksl/SkSLMetalCodeGenerator.h
index 3eccbf0909..7a30745612 100644
--- a/src/sksl/SkSLMetalCodeGenerator.h
+++ b/src/sksl/SkSLMetalCodeGenerator.h
@@ -124,8 +124,6 @@ protected:
 
     void writeModifiers(const Modifiers& modifiers, bool globalContext);
 
-    void writeGlobalVars(const VarDeclaration& vs);
-
     void writeVarInitializer(const Variable& var, const Expression& value);
 
     void writeVarDeclarations(const VarDeclarations& decl, bool global);
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 44521020e7..dc9cc8d047 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2098,7 +2098,9 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, O
                                              Modifiers(layout, Modifiers::kUniform_Flag),
                                              name,
                                              intfStruct,
-                                             Variable::kGlobal_Storage);
+                                             Variable::kGlobal_Storage,
+                                             nullptr,
+                                             {});
             fSynthetics.takeOwnership(intfVar);
             InterfaceBlock intf(-1, intfVar, name, String(""),
                                 std::vector<std::unique_ptr<Expression>>(), st);
@@ -2900,12 +2902,7 @@ void SPIRVCodeGenerator::writePrecisionModifier(const Modifiers& modifiers, SpvI
 #define BUILTIN_IGNORE 9999
 void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclarations& decl,
                                          OutputStream& out) {
-    for (size_t i = 0; i < decl.fVars.size(); i++) {
-        if (decl.fVars[i]->fKind == Statement::kNop_Kind) {
-            continue;
-        }
-        const VarDeclaration& varDecl = (VarDeclaration&) *decl.fVars[i];
-        const Variable* var = varDecl.fVar;
+    for (const Variable* var : decl.fVars) {
         // These haven't been implemented in our SPIR-V generator yet and we only currently use them
         // in the OpenGL backend.
         ASSERT(!(var->fModifiers.fFlags & (Modifiers::kReadOnly_Flag |
@@ -2949,10 +2946,11 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio
         this->writeInstruction(SpvOpVariable, type, id, storageClass, fConstantBuffer);
         this->writeInstruction(SpvOpName, id, var->fName, fNameBuffer);
         this->writePrecisionModifier(var->fModifiers, id);
-        if (varDecl.fValue) {
+        if (var->fInitialValue) {
             ASSERT(!fCurrentBlock);
             fCurrentBlock = -1;
-            SpvId value = this->writeExpression(*varDecl.fValue, fGlobalInitializersBuffer);
+            SpvId value = this->writeExpression(*var->fInitialValue,
+                                                fGlobalInitializersBuffer);
             this->writeInstruction(SpvOpStore, id, value, fGlobalInitializersBuffer);
             fCurrentBlock = 0;
         }
@@ -2968,10 +2966,7 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio
 }
 
 void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, OutputStream& out) {
-    for (const auto& stmt : decl.fVars) {
-        ASSERT(stmt->fKind == Statement::kVarDeclaration_Kind);
-        VarDeclaration& varDecl = (VarDeclaration&) *stmt;
-        const Variable* var = varDecl.fVar;
+    for (const Variable* var : decl.fVars) {
         // These haven't been implemented in our SPIR-V generator yet and we only currently use them
         // in the OpenGL backend.
         ASSERT(!(var->fModifiers.fFlags & (Modifiers::kReadOnly_Flag |
@@ -2984,8 +2979,8 @@ void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, Outpu
         SpvId type = this->getPointerType(var->fType, SpvStorageClassFunction);
         this->writeInstruction(SpvOpVariable, type, id, SpvStorageClassFunction, fVariableBuffer);
         this->writeInstruction(SpvOpName, id, var->fName, fNameBuffer);
-        if (varDecl.fValue) {
-            SpvId value = this->writeExpression(*varDecl.fValue, out);
+        if (var->fInitialValue) {
+            SpvId value = this->writeExpression(*var->fInitialValue, out);
             this->writeInstruction(SpvOpStore, id, value, out);
         }
     }
diff --git a/src/sksl/SkSLSectionAndParameterHelper.h b/src/sksl/SkSLSectionAndParameterHelper.h
index fccfff473b..89bae564cf 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.h
+++ b/src/sksl/SkSLSectionAndParameterHelper.h
@@ -42,11 +42,10 @@ public:
         for (const auto& p : program.fElements) {
             switch (p->fKind) {
                 case ProgramElement::kVar_Kind: {
-                    const VarDeclarations* decls = (const VarDeclarations*) p.get();
-                    for (const auto& raw : decls->fVars) {
-                        const VarDeclaration& decl = (VarDeclaration&) *raw;
-                        if (IsParameter(*decl.fVar)) {
-                            fParameters.push_back(decl.fVar);
+                    VarDeclarations* decls = (VarDeclarations*) p.get();
+                    for (const auto& var : decls->fVars) {
+                        if (IsParameter(*var)) {
+                            fParameters.push_back(var);
                         }
                     }
                     break;
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
index a116cc1c4c..8790765a36 100644
--- a/src/sksl/ir/SkSLStatement.h
+++ b/src/sksl/ir/SkSLStatement.h
@@ -30,7 +30,6 @@ struct Statement : public IRNode {
         kNop_Kind,
         kReturn_Kind,
         kSwitch_Kind,
-        kVarDeclaration_Kind,
         kVarDeclarations_Kind,
         kWhile_Kind
     };
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 707715f6dc..3bf1c380ef 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -16,73 +16,41 @@
 namespace SkSL {
 
 /**
- * A single variable declaration within a var declaration statement. For instance, the statement
- * 'int x = 2, y[3];' is a VarDeclarations statement containing two individual VarDeclaration
- * instances.
- */
-struct VarDeclaration : public Statement {
-    VarDeclaration(const Variable* var,
-                   std::vector<std::unique_ptr<Expression>> sizes,
-                   std::unique_ptr<Expression> value)
-    : INHERITED(var->fOffset, Statement::kVarDeclaration_Kind)
-    , fVar(var)
-    , fSizes(std::move(sizes))
-    , fValue(std::move(value)) {}
-
-    String description() const {
-        String result = fVar->fName;
-        for (const auto& size : fSizes) {
-            if (size) {
-                result += "[" + size->description() + "]";
-            } else {
-                result += "[]";
-            }
-        }
-        if (fValue) {
-            result += " = " + fValue->description();
-        }
-        return result;
-    }
-
-    const Variable* fVar;
-    std::vector<std::unique_ptr<Expression>> fSizes;
-    std::unique_ptr<Expression> fValue;
-
-    typedef Statement INHERITED;
-};
-
-/**
  * A variable declaration statement, which may consist of one or more individual variables.
  */
 struct VarDeclarations : public ProgramElement {
-    VarDeclarations(int offset, const Type* baseType,
-                    std::vector<std::unique_ptr<VarDeclaration>> vars)
+    VarDeclarations(int offset, const Type* baseType, std::vector<Variable*> vars)
     : INHERITED(offset, kVar_Kind)
-    , fBaseType(*baseType) {
-        for (auto& var : vars) {
-            fVars.push_back(std::unique_ptr<Statement>(var.release()));
-        }
-    }
+    , fBaseType(*baseType)
+    , fVars(std::move(vars)) {}
 
     String description() const override {
         if (!fVars.size()) {
             return String();
         }
-        String result = ((VarDeclaration&) *fVars[0]).fVar->fModifiers.description() +
+        String result = fVars[0]->fModifiers.description() +
                 fBaseType.description() + " ";
         String separator;
         for (const auto& var : fVars) {
             result += separator;
             separator = ", ";
-            result += var->description();
+            result += var->fName;
+            for (const auto& size : var->fSizes) {
+                if (size) {
+                    result += "[" + size->description() + "]";
+                } else {
+                    result += "[]";
+                }
+            }
+            if (var->fInitialValue) {
+                result += " = " + var->fInitialValue->description();
+            }
         }
         return result;
     }
 
     const Type& fBaseType;
-    // this *should* be a vector of unique_ptr<VarDeclaration>, but it significantly simplifies the
-    // CFG to only have to worry about unique_ptr<Statement>
-    std::vector<std::unique_ptr<Statement>> fVars;
+    std::vector<Variable*> fVars;
 
     typedef ProgramElement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index 0258e66c6e..5128f7167d 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -22,12 +22,7 @@ struct VarDeclarationsStatement : public Statement {
     , fDeclaration(std::move(decl)) {}
 
     bool isEmpty() const override {
-        for (const auto& s : fDeclaration->fVars) {
-            if (!s->isEmpty()) {
-                return false;
-            }
-        }
-        return true;
+        return !fDeclaration->fVars.size();
     }
 
     String description() const override {
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index 536d1e6b6d..a6c3560c29 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -15,6 +15,8 @@
 
 namespace SkSL {
 
+struct Expression;
+
 /**
  * Represents a variable, whether local, global, or a function parameter. This represents the
  * variable itself (the storage location), which is shared between all VariableReferences which
@@ -28,13 +30,16 @@ struct Variable : public Symbol {
     };
 
     Variable(int offset, Modifiers modifiers, StringFragment name, const Type& type,
-             Storage storage)
+             Storage storage, std::unique_ptr<Expression> initialValue,
+             std::vector<std::unique_ptr<Expression>> sizes)
     : INHERITED(offset, kVariable_Kind, name)
     , fModifiers(modifiers)
     , fType(type)
     , fStorage(storage)
     , fReadCount(0)
-    , fWriteCount(0) {}
+    , fWriteCount(0)
+    , fInitialValue(std::move(initialValue))
+    , fSizes(std::move(sizes)) {}
 
     virtual String description() const override {
         return fModifiers.description() + fType.fName + " " + fName;
@@ -55,6 +60,9 @@ struct Variable : public Symbol {
     // eliminated.
     mutable int fWriteCount;
 
+    std::unique_ptr<Expression> fInitialValue;
+    std::vector<std::unique_ptr<Expression>> fSizes;
+
     typedef Symbol INHERITED;
 };
 
diff --git a/tests/SkSLErrorTest.cpp b/tests/SkSLErrorTest.cpp
index 575fd74917..7b7d2b921c 100644
--- a/tests/SkSLErrorTest.cpp
+++ b/tests/SkSLErrorTest.cpp
@@ -326,7 +326,7 @@ DEF_TEST(SkSLUseWithoutInitialize, r) {
                  "int main() { int r; return r; }",
                  "error: 1: 'r' has not been assigned\n1 error\n");
     test_failure(r,
-                 "void main() { int x; int y = x; }",
+                 "void main() { int x; int y = x; sk_FragColor = float4(y); }",
                  "error: 1: 'x' has not been assigned\n1 error\n");
     test_failure(r,
                  "void main() { bool x; if (true && (false || x)) return; }",