Revert "refactored SkSLVarDeclaration out of existence"

This reverts commit 1ae353c887fdf447e1fe627e3cd29f8fa62c2a05.

Reason for revert: ASAN failures

Original change's description:
> 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>

TBR=egdaniel@google.com,ethannicholas@google.com

Change-Id: I8cae451de1546fe783e32dd41df00eac7da61b21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/68280
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

16 files changed, 220 insertions, 201 deletions

diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index a0282a4aca..5fd4229457 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -123,46 +123,6 @@ 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) {
@@ -506,14 +466,20 @@ 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 (Variable* var : decls.fDeclaration->fVars) {
-                if (var->fInitialValue) {
-                    this->addExpression(cfg, &var->fInitialValue, true);
+            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);
                 }
+                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 3fe0d2a47e..885d9261ec 100644
--- a/src/sksl/SkSLCFGGenerator.h
+++ b/src/sksl/SkSLCFGGenerator.h
@@ -96,14 +96,6 @@ 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 184453812a..506daa8b4f 100644
--- a/src/sksl/SkSLCPPCodeGenerator.cpp
+++ b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -273,10 +273,11 @@ 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& var : decls->fVars) {
-                    if (var == &((VariableReference&) *c.fArguments[0]).fVariable) {
+                for (const auto& raw : decls->fVars) {
+                    VarDeclaration& decl = (VarDeclaration&) *raw;
+                    if (decl.fVar == &((VariableReference&) *c.fArguments[0]).fVariable) {
                         found = true;
-                    } else if (var->fType == *fContext.fFragmentProcessor_Type) {
+                    } else if (decl.fVar->fType == *fContext.fFragmentProcessor_Type) {
                         ++index;
                     }
                 }
@@ -352,7 +353,7 @@ void CPPCodeGenerator::writeProgramElement(const ProgramElement& p) {
         if (!decls.fVars.size()) {
             return;
         }
-        const Variable& var = *decls.fVars[0];
+        const Variable& var = *((VarDeclaration&) *decls.fVars[0]).fVar;
         if (var.fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kUniform_Flag) ||
             -1 != var.fModifiers.fLayout.fBuiltin) {
             return;
@@ -412,17 +413,18 @@ 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& var : decls->fVars) {
-                if (is_private(*var)) {
-                    if (var->fType == *fContext.fFragmentProcessor_Type) {
-                        fErrors.error(var->fOffset,
+            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,
                                       "fragmentProcessor variables must be declared 'in'");
                         return;
                     }
                     this->writef("%s %s;\n",
-                                 HCodeGenerator::FieldType(fContext, var->fType,
-                                                           var->fModifiers.fLayout).c_str(),
-                                 String(var->fName).c_str());
+                                 HCodeGenerator::FieldType(fContext, decl.fVar->fType,
+                                                           decl.fVar->fModifiers.fLayout).c_str(),
+                                 String(decl.fVar->fName).c_str());
                 }
             }
         }
@@ -433,11 +435,12 @@ 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& var : decls->fVars) {
-                if (is_private(*var) && var->fInitialValue) {
-                    this->writef("%s = ", String(var->fName).c_str());
+            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());
                     fCPPMode = true;
-                    this->writeExpression(*var->fInitialValue, kAssignment_Precedence);
+                    this->writeExpression(*decl.fValue, kAssignment_Precedence);
                     fCPPMode = false;
                     this->write(";\n");
                 }
@@ -498,11 +501,12 @@ 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& var : decls->fVars) {
-                String nameString(var->fName);
+            for (const auto& raw : decls->fVars) {
+                VarDeclaration& decl = (VarDeclaration&) *raw;
+                String nameString(decl.fVar->fName);
                 const char* name = nameString.c_str();
-                if (SectionAndParameterHelper::IsParameter(*var) &&
-                    is_accessible(*var)) {
+                if (SectionAndParameterHelper::IsParameter(*decl.fVar) &&
+                    is_accessible(*decl.fVar)) {
                     this->writef("        auto %s = _outer.%s();\n"
                                  "        (void) %s;\n",
                                  name, name, name);
@@ -569,15 +573,16 @@ 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 Variable* var : decls->fVars) {
-                    String nameString(var->fName);
+                for (const auto& raw : decls->fVars) {
+                    VarDeclaration& decl = (VarDeclaration&) *raw;
+                    String nameString(decl.fVar->fName);
                     const char* name = nameString.c_str();
-                    if (needs_uniform_var(*var)) {
+                    if (needs_uniform_var(*decl.fVar)) {
                         this->writef("        UniformHandle& %s = %sVar;\n"
                                      "        (void) %s;\n",
                                      name, HCodeGenerator::FieldName(name).c_str(), name);
-                    } else if (SectionAndParameterHelper::IsParameter(*var) &&
-                               var->fType != *fContext.fFragmentProcessor_Type) {
+                    } else if (SectionAndParameterHelper::IsParameter(*decl.fVar) &&
+                               decl.fVar->fType != *fContext.fFragmentProcessor_Type) {
                         if (!wroteProcessor) {
                             this->writef("        const %s& _outer = _proc.cast<%s>();\n", fullName,
                                          fullName);
@@ -713,10 +718,11 @@ bool CPPCodeGenerator::generateCode() {
     for (const auto& p : fProgram.fElements) {
         if (ProgramElement::kVar_Kind == p->fKind) {
             const VarDeclarations* decls = (const VarDeclarations*) p.get();
-            for (const Variable* var : decls->fVars) {
-                if ((var->fModifiers.fFlags & Modifiers::kUniform_Flag) &&
-                           var->fType.kind() != Type::kSampler_Kind) {
-                    uniforms.push_back(var);
+            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);
                 }
             }
         }
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 1b04bb1dea..0b51824f0d 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -189,14 +189,12 @@ Compiler::Compiler(Flags flags)
 
     StringFragment skCapsName("sk_Caps");
     Variable* skCaps = new Variable(-1, Modifiers(), skCapsName,
-                                    *fContext.fSkCaps_Type, Variable::kGlobal_Storage, nullptr,
-                                    {});
+                                    *fContext.fSkCaps_Type, Variable::kGlobal_Storage);
     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, nullptr,
-                                    {});
+                                    *fContext.fSkArgs_Type, Variable::kGlobal_Storage);
     fIRGenerator->fSymbolTable->add(skArgsName, std::unique_ptr<Symbol>(skArgs));
 
     std::vector<std::unique_ptr<ProgramElement>> ignored;
@@ -309,11 +307,10 @@ void Compiler::addDefinitions(const BasicBlock::Node& node,
         }
         case BasicBlock::Node::kStatement_Kind: {
             const Statement* stmt = (Statement*) node.statement()->get();
-            if (stmt->fKind == Statement::kVarDeclarations_Kind) {
-                for (Variable* var : ((VarDeclarationsStatement*) stmt)->fDeclaration->fVars) {
-                    if (var->fInitialValue) {
-                        (*definitions)[var] = &var->fInitialValue;
-                    }
+            if (stmt->fKind == Statement::kVarDeclaration_Kind) {
+                VarDeclaration& vd = (VarDeclaration&) *stmt;
+                if (vd.fValue) {
+                    (*definitions)[vd.fVar] = &vd.fValue;
                 }
             }
             break;
@@ -369,8 +366,10 @@ 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 Variable* var : vd->fDeclaration->fVars) {
-                        result[var] = nullptr;
+                    for (const auto& decl : vd->fDeclaration->fVars) {
+                        if (decl->fKind == Statement::kVarDeclaration_Kind) {
+                            result[((VarDeclaration&) *decl).fVar] = nullptr;
+                        }
                     }
                 }
             }
@@ -645,7 +644,8 @@ 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,25 +890,19 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
                                  bool* outNeedsRescan) {
     Statement* stmt = (*iter)->statement()->get();
     switch (stmt->fKind) {
-        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;
-                        }
+        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;
                     }
-                    varIter = vars.erase(varIter);
-                    *outUpdated = true;
-                } else {
-                    ++varIter;
                 }
-            }
-            if (!vars.size()) {
                 (*iter)->setStatement(std::unique_ptr<Statement>(new Nop()));
+                *outUpdated = true;
             }
             break;
         }
@@ -1100,6 +1094,22 @@ 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 e3c2003a3f..22331cdc80 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -789,29 +789,30 @@ void GLSLCodeGenerator::writeTypePrecision(const Type& type) {
 void GLSLCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool global) {
     ASSERT(decl.fVars.size() > 0);
     bool wroteType = false;
-    for (const Variable* var : decl.fVars) {
+    for (const auto& stmt : decl.fVars) {
+        VarDeclaration& var = (VarDeclaration&) *stmt;
         if (wroteType) {
             this->write(", ");
         } else {
-            this->writeModifiers(var->fModifiers, global);
+            this->writeModifiers(var.fVar->fModifiers, global);
             this->writeTypePrecision(decl.fBaseType);
             this->writeType(decl.fBaseType);
             this->write(" ");
             wroteType = true;
         }
-        this->write(var->fName);
-        for (const auto& size : var->fSizes) {
+        this->write(var.fVar->fName);
+        for (const auto& size : var.fSizes) {
             this->write("[");
             if (size) {
                 this->writeExpression(*size, kTopLevel_Precedence);
             }
             this->write("]");
         }
-        if (var->fInitialValue) {
+        if (var.fValue) {
             this->write(" = ");
-            this->writeVarInitializer(*var, *var->fInitialValue);
+            this->writeVarInitializer(*var.fVar, *var.fValue);
         }
-        if (!fFoundImageDecl && var->fType == *fContext.fImage2D_Type) {
+        if (!fFoundImageDecl && var.fVar->fType == *fContext.fImage2D_Type) {
             if (fProgram.fSettings.fCaps->imageLoadStoreExtensionString()) {
                 fHeader.writeText("#extension ");
                 fHeader.writeText(fProgram.fSettings.fCaps->imageLoadStoreExtensionString());
@@ -983,7 +984,7 @@ void GLSLCodeGenerator::writeProgramElement(const ProgramElement& e) {
         case ProgramElement::kVar_Kind: {
             VarDeclarations& decl = (VarDeclarations&) e;
             if (decl.fVars.size() > 0) {
-                int builtin = decl.fVars[0]->fModifiers.fLayout.fBuiltin;
+                int builtin = ((VarDeclaration&) *decl.fVars[0]).fVar->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 5e1dffe70a..e9a63ac48f 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.h
+++ b/src/sksl/SkSLGLSLCodeGenerator.h
@@ -116,6 +116,8 @@ 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 7bbb6da362..2fb27fcec4 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<Variable*> variables;
+    std::vector<std::unique_ptr<VarDeclaration>> variables;
     const Type* baseType = this->convertType(*decl.fType);
     if (!baseType) {
         return nullptr;
@@ -254,6 +254,8 @@ 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);
@@ -261,11 +263,6 @@ 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" &&
@@ -278,7 +275,8 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa
             Variable* old = (Variable*) (*fSymbolTable)[varDecl.fName];
             old->fModifiers = var->fModifiers;
         } else {
-            variables.push_back(var.get());
+            variables.emplace_back(new VarDeclaration(var.get(), std::move(sizes),
+                                                      std::move(value)));
             fSymbolTable->add(varDecl.fName, std::move(var));
         }
     }
@@ -533,7 +531,7 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
                                                                           std::move(main))));
     fSymbolTable->add(invokeDecl->fName, std::unique_ptr<FunctionDeclaration>(invokeDecl));
 
-    std::vector<Variable*> variables;
+    std::vector<std::unique_ptr<VarDeclaration>> variables;
     Variable* loopIdx = (Variable*) (*fSymbolTable)["sk_InvocationID"];
     ASSERT(loopIdx);
     std::unique_ptr<Expression> test(new BinaryExpression(-1,
@@ -600,7 +598,7 @@ void IRGenerator::convertFunction(const ASTFunction& f,
         }
         StringFragment name = param->fName;
         Variable* var = new Variable(param->fOffset, param->fModifiers, name, *type,
-                                     Variable::kParameter_Storage, nullptr, {});
+                                     Variable::kParameter_Storage);
         fSymbolTable->takeOwnership(var);
         parameters.push_back(var);
     }
@@ -714,27 +712,29 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte
         if (!decl) {
             return nullptr;
         }
-        for (const Variable* var : decl->fVars) {
+        for (const auto& stmt : decl->fVars) {
+            VarDeclaration& vd = (VarDeclaration&) *stmt;
             if (haveRuntimeArray) {
-                fErrors.error(var->fOffset,
+                fErrors.error(decl->fOffset,
                               "only the last entry in an interface block may be a runtime-sized "
                               "array");
             }
-            fields.push_back(Type::Field(var->fModifiers, var->fName, &var->fType));
-            if (var->fInitialValue) {
+            fields.push_back(Type::Field(vd.fVar->fModifiers, vd.fVar->fName,
+                                         &vd.fVar->fType));
+            if (vd.fValue) {
                 fErrors.error(decl->fOffset,
                               "initializers are not permitted on interface block fields");
             }
-            if (var->fModifiers.fFlags & (Modifiers::kIn_Flag |
-                                          Modifiers::kOut_Flag |
-                                          Modifiers::kUniform_Flag |
-                                          Modifiers::kBuffer_Flag |
-                                          Modifiers::kConst_Flag)) {
+            if (vd.fVar->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 (var->fType.kind() == Type::kArray_Kind &&
-                var->fType.columns() == -1) {
+            if (vd.fVar->fType.kind() == Type::kArray_Kind &&
+                vd.fVar->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, nullptr, {});
+                                 *type, Variable::kGlobal_Storage);
     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 713c28a3ed..de6b1d0854 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -538,33 +538,34 @@ 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 Variable* var : decl.fVars) {
-        if (var->fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag |
-                                      Modifiers::kUniform_Flag)) {
+    for (const auto& stmt : decl.fVars) {
+        VarDeclaration& var = (VarDeclaration&) *stmt;
+        if (var.fVar->fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag |
+                                           Modifiers::kUniform_Flag)) {
             ASSERT(global);
             continue;
         }
         if (wroteType) {
             this->write(", ");
         } else {
-            this->writeModifiers(var->fModifiers, global);
+            this->writeModifiers(var.fVar->fModifiers, global);
             this->writeType(decl.fBaseType);
             this->write(" ");
             wroteType = true;
         }
-        this->write(var->fName);
-        for (const auto& size : var->fSizes) {
+        this->write(var.fVar->fName);
+        for (const auto& size : var.fSizes) {
             this->write("[");
             if (size) {
                 this->writeExpression(*size, kTopLevel_Precedence);
             }
             this->write("]");
         }
-        if (var->fInitialValue) {
+        if (var.fValue) {
             this->write(" = ");
-            this->writeVarInitializer(*var, *var->fInitialValue);
+            this->writeVarInitializer(*var.fVar, *var.fValue);
         }
-        if (!fFoundImageDecl && var->fType == *fContext.fImage2D_Type) {
+        if (!fFoundImageDecl && var.fVar->fType == *fContext.fImage2D_Type) {
             if (fProgram.fSettings.fCaps->imageLoadStoreExtensionString()) {
                 fHeader.writeText("#extension ");
                 fHeader.writeText(fProgram.fSettings.fCaps->imageLoadStoreExtensionString());
@@ -732,7 +733,7 @@ void MetalCodeGenerator::writeUniformStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *decls.fVars[0];
+            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
             if (first.fModifiers.fFlags & Modifiers::kUniform_Flag) {
                 if (-1 == fUniformBuffer) {
                     this->write("struct Uniforms {\n");
@@ -749,8 +750,9 @@ void MetalCodeGenerator::writeUniformStruct() {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const auto& var : decls.fVars) {
-                    this->write(var->fName);
+                for (const auto& stmt : decls.fVars) {
+                    VarDeclaration& var = (VarDeclaration&) *stmt;
+                    this->write(var.fVar->fName);
                 }
                 this->write(";\n");
             }
@@ -772,17 +774,18 @@ void MetalCodeGenerator::writeInputStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *decls.fVars[0];
+            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
             if (first.fModifiers.fFlags & Modifiers::kIn_Flag &&
                 -1 == first.fModifiers.fLayout.fBuiltin) {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const Variable* var : decls.fVars) {
-                    this->write(var->fName);
-                    if (-1 != var->fModifiers.fLayout.fLocation) {
+                for (const auto& stmt : decls.fVars) {
+                    VarDeclaration& var = (VarDeclaration&) *stmt;
+                    this->write(var.fVar->fName);
+                    if (-1 != var.fVar->fModifiers.fLayout.fLocation) {
                         this->write("  [[attribute(" +
-                                    to_string(var->fModifiers.fLayout.fLocation) + ")]]");
+                                    to_string(var.fVar->fModifiers.fLayout.fLocation) + ")]]");
                     }
                 }
                 this->write(";\n");
@@ -801,14 +804,15 @@ void MetalCodeGenerator::writeOutputStruct() {
             if (!decls.fVars.size()) {
                 continue;
             }
-            const Variable& first = *decls.fVars[0];
+            const Variable& first = *((VarDeclaration&) *decls.fVars[0]).fVar;
             if (first.fModifiers.fFlags & Modifiers::kOut_Flag &&
                 -1 == first.fModifiers.fLayout.fBuiltin) {
                 this->write("    ");
                 this->writeType(first.fType);
                 this->write(" ");
-                for (const Variable* var : decls.fVars) {
-                    this->write(var->fName);
+                for (const auto& stmt : decls.fVars) {
+                    VarDeclaration& var = (VarDeclaration&) *stmt;
+                    this->write(var.fVar->fName);
                 }
                 this->write(";\n");
             }
@@ -823,7 +827,7 @@ void MetalCodeGenerator::writeProgramElement(const ProgramElement& e) {
         case ProgramElement::kVar_Kind: {
             VarDeclarations& decl = (VarDeclarations&) e;
             if (decl.fVars.size() > 0) {
-                int builtin = decl.fVars[0]->fModifiers.fLayout.fBuiltin;
+                int builtin = ((VarDeclaration&) *decl.fVars[0]).fVar->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 7a30745612..3eccbf0909 100644
--- a/src/sksl/SkSLMetalCodeGenerator.h
+++ b/src/sksl/SkSLMetalCodeGenerator.h
@@ -124,6 +124,8 @@ 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 dc9cc8d047..44521020e7 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2098,9 +2098,7 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, O
                                              Modifiers(layout, Modifiers::kUniform_Flag),
                                              name,
                                              intfStruct,
-                                             Variable::kGlobal_Storage,
-                                             nullptr,
-                                             {});
+                                             Variable::kGlobal_Storage);
             fSynthetics.takeOwnership(intfVar);
             InterfaceBlock intf(-1, intfVar, name, String(""),
                                 std::vector<std::unique_ptr<Expression>>(), st);
@@ -2902,7 +2900,12 @@ void SPIRVCodeGenerator::writePrecisionModifier(const Modifiers& modifiers, SpvI
 #define BUILTIN_IGNORE 9999
 void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclarations& decl,
                                          OutputStream& out) {
-    for (const Variable* var : decl.fVars) {
+    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;
         // 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 |
@@ -2946,11 +2949,10 @@ 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 (var->fInitialValue) {
+        if (varDecl.fValue) {
             ASSERT(!fCurrentBlock);
             fCurrentBlock = -1;
-            SpvId value = this->writeExpression(*var->fInitialValue,
-                                                fGlobalInitializersBuffer);
+            SpvId value = this->writeExpression(*varDecl.fValue, fGlobalInitializersBuffer);
             this->writeInstruction(SpvOpStore, id, value, fGlobalInitializersBuffer);
             fCurrentBlock = 0;
         }
@@ -2966,7 +2968,10 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio
 }
 
 void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, OutputStream& out) {
-    for (const Variable* var : decl.fVars) {
+    for (const auto& stmt : decl.fVars) {
+        ASSERT(stmt->fKind == Statement::kVarDeclaration_Kind);
+        VarDeclaration& varDecl = (VarDeclaration&) *stmt;
+        const Variable* var = varDecl.fVar;
         // 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 |
@@ -2979,8 +2984,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 (var->fInitialValue) {
-            SpvId value = this->writeExpression(*var->fInitialValue, out);
+        if (varDecl.fValue) {
+            SpvId value = this->writeExpression(*varDecl.fValue, out);
             this->writeInstruction(SpvOpStore, id, value, out);
         }
     }
diff --git a/src/sksl/SkSLSectionAndParameterHelper.h b/src/sksl/SkSLSectionAndParameterHelper.h
index 89bae564cf..fccfff473b 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.h
+++ b/src/sksl/SkSLSectionAndParameterHelper.h
@@ -42,10 +42,11 @@ public:
         for (const auto& p : program.fElements) {
             switch (p->fKind) {
                 case ProgramElement::kVar_Kind: {
-                    VarDeclarations* decls = (VarDeclarations*) p.get();
-                    for (const auto& var : decls->fVars) {
-                        if (IsParameter(*var)) {
-                            fParameters.push_back(var);
+                    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);
                         }
                     }
                     break;
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
index 8790765a36..a116cc1c4c 100644
--- a/src/sksl/ir/SkSLStatement.h
+++ b/src/sksl/ir/SkSLStatement.h
@@ -30,6 +30,7 @@ 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 3bf1c380ef..707715f6dc 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -16,41 +16,73 @@
 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<Variable*> vars)
+    VarDeclarations(int offset, const Type* baseType,
+                    std::vector<std::unique_ptr<VarDeclaration>> vars)
     : INHERITED(offset, kVar_Kind)
-    , fBaseType(*baseType)
-    , fVars(std::move(vars)) {}
+    , fBaseType(*baseType) {
+        for (auto& var : vars) {
+            fVars.push_back(std::unique_ptr<Statement>(var.release()));
+        }
+    }
 
     String description() const override {
         if (!fVars.size()) {
             return String();
         }
-        String result = fVars[0]->fModifiers.description() +
+        String result = ((VarDeclaration&) *fVars[0]).fVar->fModifiers.description() +
                 fBaseType.description() + " ";
         String separator;
         for (const auto& var : fVars) {
             result += separator;
             separator = ", ";
-            result += var->fName;
-            for (const auto& size : var->fSizes) {
-                if (size) {
-                    result += "[" + size->description() + "]";
-                } else {
-                    result += "[]";
-                }
-            }
-            if (var->fInitialValue) {
-                result += " = " + var->fInitialValue->description();
-            }
+            result += var->description();
         }
         return result;
     }
 
     const Type& fBaseType;
-    std::vector<Variable*> fVars;
+    // 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;
 
     typedef ProgramElement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index 5128f7167d..0258e66c6e 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -22,7 +22,12 @@ struct VarDeclarationsStatement : public Statement {
     , fDeclaration(std::move(decl)) {}
 
     bool isEmpty() const override {
-        return !fDeclaration->fVars.size();
+        for (const auto& s : fDeclaration->fVars) {
+            if (!s->isEmpty()) {
+                return false;
+            }
+        }
+        return true;
     }
 
     String description() const override {
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index a6c3560c29..536d1e6b6d 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -15,8 +15,6 @@
 
 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
@@ -30,16 +28,13 @@ struct Variable : public Symbol {
     };
 
     Variable(int offset, Modifiers modifiers, StringFragment name, const Type& type,
-             Storage storage, std::unique_ptr<Expression> initialValue,
-             std::vector<std::unique_ptr<Expression>> sizes)
+             Storage storage)
     : INHERITED(offset, kVariable_Kind, name)
     , fModifiers(modifiers)
     , fType(type)
     , fStorage(storage)
     , fReadCount(0)
-    , fWriteCount(0)
-    , fInitialValue(std::move(initialValue))
-    , fSizes(std::move(sizes)) {}
+    , fWriteCount(0) {}
 
     virtual String description() const override {
         return fModifiers.description() + fType.fName + " " + fName;
@@ -60,9 +55,6 @@ 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 7b7d2b921c..575fd74917 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; sk_FragColor = float4(y); }",
+                 "void main() { int x; int y = x; }",
                  "error: 1: 'x' has not been assigned\n1 error\n");
     test_failure(r,
                  "void main() { bool x; if (true && (false || x)) return; }",