Added dead variable / code elimination to skslc.

BUG=skia:

Change-Id: Ib037730803a8f222f099de0e001fe06ad452a22c
Reviewed-on: https://skia-review.googlesource.com/7584
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ben Wagner <benjaminwagner@google.com>

27 files changed, 194 insertions, 40 deletions

diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index de85e4812b..4837d18093 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -34,6 +34,11 @@ struct BinaryExpression : public Expression {
                                         *fRight);
     }
 
+    virtual bool hasSideEffects() const override {
+        return Token::IsAssignment(fOperator) || fLeft->hasSideEffects() ||
+               fRight->hasSideEffects();
+    }
+
     virtual SkString description() const override {
         return "(" + fLeft->description() + " " + Token::OperatorName(fOperator) + " " +
                fRight->description() + ")";
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index 17970fd561..f00c146725 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -23,6 +23,15 @@ struct Block : public Statement {
     , fSymbols(std::move(symbols))
     , fStatements(std::move(statements)) {}
 
+    virtual bool isEmpty() const override {
+        for (const auto& s : fStatements) {
+            if (!s->isEmpty()) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     SkString description() const override {
         SkString result("{");
         for (size_t i = 0; i < fStatements.size(); i++) {
@@ -36,7 +45,7 @@ struct Block : public Statement {
     // it's important to keep fStatements defined after (and thus destroyed before) fSymbols,
     // because destroying statements can modify reference counts in symbols
     const std::shared_ptr<SymbolTable> fSymbols;
-    const std::vector<std::unique_ptr<Statement>> fStatements;
+    std::vector<std::unique_ptr<Statement>> fStatements;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLBoolLiteral.h b/src/sksl/ir/SkSLBoolLiteral.h
index b372f2f3ff..5ca7c7428a 100644
--- a/src/sksl/ir/SkSLBoolLiteral.h
+++ b/src/sksl/ir/SkSLBoolLiteral.h
@@ -25,6 +25,10 @@ struct BoolLiteral : public Expression {
         return SkString(fValue ? "true" : "false");
     }
 
+    bool hasSideEffects() const override {
+        return false;
+    }
+
     bool isConstant() const override {
         return true;
     }
diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h
index 691bea123a..5ecb74eb72 100644
--- a/src/sksl/ir/SkSLConstructor.h
+++ b/src/sksl/ir/SkSLConstructor.h
@@ -24,20 +24,36 @@ struct Constructor : public Expression {
     : INHERITED(position, kConstructor_Kind, type)
     , fArguments(std::move(arguments)) {}
 
-    virtual std::unique_ptr<Expression> constantPropagate(
-                                                        const IRGenerator& irGenerator,
-                                                        const DefinitionMap& definitions) override {
-        if (fArguments.size() == 1 && fArguments[0]->fKind == Expression::kIntLiteral_Kind &&
-            // promote float(1) to 1.0
-            fType == *irGenerator.fContext.fFloat_Type) {
-            int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue;
-            return std::unique_ptr<Expression>(new FloatLiteral(irGenerator.fContext,
-                                                                fPosition,
-                                                                intValue));
+    std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
+                                                  const DefinitionMap& definitions) override {
+        if (fArguments.size() == 1 && fArguments[0]->fKind == Expression::kIntLiteral_Kind) {
+            if (fType == *irGenerator.fContext.fFloat_Type) {
+                // promote float(1) to 1.0
+                int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue;
+                return std::unique_ptr<Expression>(new FloatLiteral(irGenerator.fContext,
+                                                                    fPosition,
+                                                                    intValue));
+            } else if (fType == *irGenerator.fContext.fUInt_Type) {
+                // promote uint(1) to 1u
+                int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue;
+                return std::unique_ptr<Expression>(new IntLiteral(irGenerator.fContext,
+                                                                  fPosition,
+                                                                  intValue,
+                                                                  &fType));
+            }
         }
         return nullptr;
     }
 
+    bool hasSideEffects() const override {
+        for (const auto& arg : fArguments) {
+            if (arg->hasSideEffects()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     SkString description() const override {
         SkString result = fType.description() + "(";
         SkString separator;
diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h
index e26d3dc974..1b233f9889 100644
--- a/src/sksl/ir/SkSLDoStatement.h
+++ b/src/sksl/ir/SkSLDoStatement.h
@@ -27,7 +27,7 @@ struct DoStatement : public Statement {
         return "do " + fStatement->description() + " while (" + fTest->description() + ");";
     }
 
-    const std::unique_ptr<Statement> fStatement;
+    std::unique_ptr<Statement> fStatement;
     std::unique_ptr<Expression> fTest;
 
     typedef Statement INHERITED;
diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h
index f87d810fc0..5db9ddf96f 100644
--- a/src/sksl/ir/SkSLExpression.h
+++ b/src/sksl/ir/SkSLExpression.h
@@ -53,6 +53,13 @@ struct Expression : public IRNode {
     }
 
     /**
+     * Returns true if evaluating the expression potentially has side effects. Expressions may never
+     * return false if they actually have side effects, but it is legal (though suboptimal) to
+     * return true if there are not actually any side effects.
+     */
+    virtual bool hasSideEffects() const = 0;
+
+    /**
      * Given a map of known constant variable values, substitute them in for references to those
      * variables occurring in this expression and its subexpressions.  Similar simplifications, such
      * as folding a constant binary expression down to a single value, may also be performed.
diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h
index de26a3f626..b4f5695b63 100644
--- a/src/sksl/ir/SkSLFieldAccess.h
+++ b/src/sksl/ir/SkSLFieldAccess.h
@@ -24,14 +24,18 @@ struct FieldAccess : public Expression {
         kAnonymousInterfaceBlock_OwnerKind
     };
 
-    FieldAccess(std::unique_ptr<Expression> base, int fieldIndex, 
+    FieldAccess(std::unique_ptr<Expression> base, int fieldIndex,
                 OwnerKind ownerKind = kDefault_OwnerKind)
     : INHERITED(base->fPosition, kFieldAccess_Kind, *base->fType.fields()[fieldIndex].fType)
     , fBase(std::move(base))
     , fFieldIndex(fieldIndex)
     , fOwnerKind(ownerKind) {}
 
-    virtual SkString description() const override {
+    bool hasSideEffects() const override {
+        return fBase->hasSideEffects();
+    }
+
+    SkString description() const override {
         return fBase->description() + "." + fBase->fType.fields()[fFieldIndex].fName;
     }
 
diff --git a/src/sksl/ir/SkSLFloatLiteral.h b/src/sksl/ir/SkSLFloatLiteral.h
index 8a1a5ad63a..8fdf1af603 100644
--- a/src/sksl/ir/SkSLFloatLiteral.h
+++ b/src/sksl/ir/SkSLFloatLiteral.h
@@ -21,10 +21,14 @@ struct FloatLiteral : public Expression {
     : INHERITED(position, kFloatLiteral_Kind, *context.fFloat_Type)
     , fValue(value) {}
 
-    virtual SkString description() const override {
+    SkString description() const override {
         return to_string(fValue);
     }
 
+    bool hasSideEffects() const override {
+        return false;
+    }
+
     bool isConstant() const override {
         return true;
     }
diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h
index f2bf880ddd..4f2228a42f 100644
--- a/src/sksl/ir/SkSLForStatement.h
+++ b/src/sksl/ir/SkSLForStatement.h
@@ -48,10 +48,10 @@ struct ForStatement : public Statement {
     // it's important to keep fSymbols defined first (and thus destroyed last) because destroying
     // the other fields can update symbol reference counts
     const std::shared_ptr<SymbolTable> fSymbols;
-    const std::unique_ptr<Statement> fInitializer;
+    std::unique_ptr<Statement> fInitializer;
     std::unique_ptr<Expression> fTest;
     std::unique_ptr<Expression> fNext;
-    const std::unique_ptr<Statement> fStatement;
+    std::unique_ptr<Statement> fStatement;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index 1838076796..73a174bc2d 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -23,6 +23,21 @@ struct FunctionCall : public Expression {
     , fFunction(std::move(function))
     , fArguments(std::move(arguments)) {}
 
+    bool hasSideEffects() const override {
+        if (!fFunction.fBuiltin) {
+            // conservatively assume that user-defined functions have side effects
+            return true;
+        }
+        for (const auto& arg : fArguments) {
+            if (arg->hasSideEffects()) {
+                return true;
+            }
+        }
+        // Note that we are assuming no builtin functions have side effects. This is true for the
+        // moment, but could change as our support for GLSL functions expands.
+        return false;
+    }
+
     SkString description() const override {
         SkString result = fFunction.fName + "(";
         SkString separator;
diff --git a/src/sksl/ir/SkSLFunctionDefinition.h b/src/sksl/ir/SkSLFunctionDefinition.h
index bae882525a..7e7b115188 100644
--- a/src/sksl/ir/SkSLFunctionDefinition.h
+++ b/src/sksl/ir/SkSLFunctionDefinition.h
@@ -18,8 +18,8 @@ namespace SkSL {
  * A function definition (a declaration plus an associated block of code).
  */
 struct FunctionDefinition : public ProgramElement {
-    FunctionDefinition(Position position, const FunctionDeclaration& declaration, 
-                       std::unique_ptr<Block> body)
+    FunctionDefinition(Position position, const FunctionDeclaration& declaration,
+                       std::unique_ptr<Statement> body)
     : INHERITED(position, kFunction_Kind)
     , fDeclaration(declaration)
     , fBody(std::move(body)) {}
@@ -29,7 +29,7 @@ struct FunctionDefinition : public ProgramElement {
     }
 
     const FunctionDeclaration& fDeclaration;
-    const std::unique_ptr<Block> fBody;
+    std::unique_ptr<Statement> fBody;
 
     typedef ProgramElement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLFunctionReference.h b/src/sksl/ir/SkSLFunctionReference.h
index ec1fc3804c..1465de9bf3 100644
--- a/src/sksl/ir/SkSLFunctionReference.h
+++ b/src/sksl/ir/SkSLFunctionReference.h
@@ -23,6 +23,10 @@ struct FunctionReference : public Expression {
     : INHERITED(position, kFunctionReference_Kind, *context.fInvalid_Type)
     , fFunctions(function) {}
 
+    bool hasSideEffects() const override {
+        return false;
+    }
+
     virtual SkString description() const override {
         ASSERT(false);
         return SkString("<function>");
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 8667e932ec..ee2612c305 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -33,8 +33,9 @@ struct IfStatement : public Statement {
     }
 
     std::unique_ptr<Expression> fTest;
-    const std::unique_ptr<Statement> fIfTrue;
-    const std::unique_ptr<Statement> fIfFalse;
+    std::unique_ptr<Statement> fIfTrue;
+    // may be null
+    std::unique_ptr<Statement> fIfFalse;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h
index d255c7daf6..0b6879378d 100644
--- a/src/sksl/ir/SkSLIndexExpression.h
+++ b/src/sksl/ir/SkSLIndexExpression.h
@@ -43,7 +43,7 @@ static const Type& index_type(const Context& context, const Type& type) {
  * An expression which extracts a value from an array or matrix, as in 'm[2]'.
  */
 struct IndexExpression : public Expression {
-    IndexExpression(const Context& context, std::unique_ptr<Expression> base, 
+    IndexExpression(const Context& context, std::unique_ptr<Expression> base,
                     std::unique_ptr<Expression> index)
     : INHERITED(base->fPosition, kIndex_Kind, index_type(context, base->fType))
     , fBase(std::move(base))
@@ -51,6 +51,10 @@ struct IndexExpression : public Expression {
         ASSERT(fIndex->fType == *context.fInt_Type || fIndex->fType == *context.fUInt_Type);
     }
 
+    bool hasSideEffects() const override {
+        return fBase->hasSideEffects() || fIndex->hasSideEffects();
+    }
+
     SkString description() const override {
         return fBase->description() + "[" + fIndex->description() + "]";
     }
diff --git a/src/sksl/ir/SkSLIntLiteral.h b/src/sksl/ir/SkSLIntLiteral.h
index 23325e65fb..2488a6a1dc 100644
--- a/src/sksl/ir/SkSLIntLiteral.h
+++ b/src/sksl/ir/SkSLIntLiteral.h
@@ -22,11 +22,15 @@ struct IntLiteral : public Expression {
     : INHERITED(position, kIntLiteral_Kind, type ? *type : *context.fInt_Type)
     , fValue(value) {}
 
-    virtual SkString description() const override {
+    SkString description() const override {
         return to_string(fValue);
     }
 
-   bool isConstant() const override {
+    bool hasSideEffects() const override {
+        return false;
+    }
+
+    bool isConstant() const override {
         return true;
     }
 
diff --git a/src/sksl/ir/SkSLNop.h b/src/sksl/ir/SkSLNop.h
new file mode 100644
index 0000000000..6b13e140b6
--- /dev/null
+++ b/src/sksl/ir/SkSLNop.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SKSL_NOP
+#define SKSL_NOP
+
+#include "SkSLStatement.h"
+#include "SkSLSymbolTable.h"
+
+namespace SkSL {
+
+/**
+ * A no-op statement that does nothing.
+ */
+struct Nop : public Statement {
+    Nop()
+    : INHERITED(Position(), kNop_Kind) {}
+
+    virtual bool isEmpty() const override {
+        return true;
+    }
+
+    SkString description() const override {
+        return SkString(";");
+    }
+
+    typedef Statement INHERITED;
+};
+
+} // namespace
+
+#endif
diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h
index 6c9fafe5a0..1c400f9fee 100644
--- a/src/sksl/ir/SkSLPostfixExpression.h
+++ b/src/sksl/ir/SkSLPostfixExpression.h
@@ -21,7 +21,11 @@ struct PostfixExpression : public Expression {
     , fOperand(std::move(operand))
     , fOperator(op) {}
 
-    virtual SkString description() const override {
+    bool hasSideEffects() const override {
+        return true;
+    }
+
+    SkString description() const override {
         return fOperand->description() + Token::OperatorName(fOperator);
     }
 
diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h
index b7db99a0a4..6138bb523b 100644
--- a/src/sksl/ir/SkSLPrefixExpression.h
+++ b/src/sksl/ir/SkSLPrefixExpression.h
@@ -21,7 +21,12 @@ struct PrefixExpression : public Expression {
     , fOperand(std::move(operand))
     , fOperator(op) {}
 
-    virtual SkString description() const override {
+    bool hasSideEffects() const override {
+        return fOperator == Token::PLUSPLUS || fOperator == Token::MINUSMINUS ||
+               fOperand->hasSideEffects();
+    }
+
+    SkString description() const override {
         return Token::OperatorName(fOperator) + fOperand->description();
     }
 
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
index 012311fdd3..02caac1f6c 100644
--- a/src/sksl/ir/SkSLStatement.h
+++ b/src/sksl/ir/SkSLStatement.h
@@ -25,7 +25,9 @@ struct Statement : public IRNode {
         kDo_Kind,
         kExpression_Kind,
         kFor_Kind,
+        kGroup_Kind,
         kIf_Kind,
+        kNop_Kind,
         kReturn_Kind,
         kVarDeclarations_Kind,
         kWhile_Kind
@@ -35,6 +37,10 @@ struct Statement : public IRNode {
     : INHERITED(position)
     , fKind(kind) {}
 
+    virtual bool isEmpty() const {
+        return false;
+    }
+
     const Kind fKind;
 
     typedef IRNode INHERITED;
diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h
index 8ad9001ada..a77397aa14 100644
--- a/src/sksl/ir/SkSLSwizzle.h
+++ b/src/sksl/ir/SkSLSwizzle.h
@@ -68,6 +68,10 @@ struct Swizzle : public Expression {
         ASSERT(fComponents.size() >= 1 && fComponents.size() <= 4);
     }
 
+    bool hasSideEffects() const override {
+        return fBase->hasSideEffects();
+    }
+
     SkString description() const override {
         SkString result = fBase->description() + ".";
         for (int x : fComponents) {
diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h
index 02750049d4..a9e8560aa1 100644
--- a/src/sksl/ir/SkSLTernaryExpression.h
+++ b/src/sksl/ir/SkSLTernaryExpression.h
@@ -26,8 +26,12 @@ struct TernaryExpression : public Expression {
         ASSERT(fIfTrue->fType == fIfFalse->fType);
     }
 
+    bool hasSideEffects() const override {
+        return fTest->hasSideEffects() || fIfTrue->hasSideEffects() || fIfFalse->hasSideEffects();
+    }
+
     SkString description() const override {
-        return "(" + fTest->description() + " ? " + fIfTrue->description() + " : " + 
+        return "(" + fTest->description() + " ? " + fIfTrue->description() + " : " +
                fIfFalse->description() + ")";
     }
 
diff --git a/src/sksl/ir/SkSLTypeReference.h b/src/sksl/ir/SkSLTypeReference.h
index 1c6f16ce58..1ee216d0d1 100644
--- a/src/sksl/ir/SkSLTypeReference.h
+++ b/src/sksl/ir/SkSLTypeReference.h
@@ -22,6 +22,10 @@ struct TypeReference : public Expression {
     : INHERITED(position, kTypeReference_Kind, *context.fInvalid_Type)
     , fValue(type) {}
 
+    bool hasSideEffects() const override {
+        return false;
+    }
+
     SkString description() const override {
         return fValue.name();
     }
diff --git a/src/sksl/ir/SkSLUnresolvedFunction.h b/src/sksl/ir/SkSLUnresolvedFunction.h
index 76741cfca8..4047df63ef 100644
--- a/src/sksl/ir/SkSLUnresolvedFunction.h
+++ b/src/sksl/ir/SkSLUnresolvedFunction.h
@@ -26,7 +26,7 @@ struct UnresolvedFunction : public Symbol {
 #endif
     }
 
-    virtual SkString description() const override {
+    SkString description() const override {
         return fName;
     }
 
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 490259a081..498a32d46e 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -52,7 +52,7 @@ struct VarDeclaration {
  */
 struct VarDeclarations : public ProgramElement {
     VarDeclarations(Position position, const Type* baseType, 
-                    std::vector<VarDeclaration> vars)
+                    std::vector<std::unique_ptr<VarDeclaration>> vars)
     : INHERITED(position, kVar_Kind)
     , fBaseType(*baseType)
     , fVars(std::move(vars)) {}
@@ -61,18 +61,18 @@ struct VarDeclarations : public ProgramElement {
         if (!fVars.size()) {
             return SkString();
         }
-        SkString result = fVars[0].fVar->fModifiers.description() + fBaseType.description() + " ";
+        SkString result = fVars[0]->fVar->fModifiers.description() + fBaseType.description() + " ";
         SkString separator;
         for (const auto& var : fVars) {
             result += separator;
             separator = ", ";
-            result += var.description();
+            result += var->description();
         }
         return result;
     }
 
     const Type& fBaseType;
-    std::vector<VarDeclaration> fVars;
+    std::vector<std::unique_ptr<VarDeclaration>> fVars;
 
     typedef ProgramElement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index 2c3391dfa2..8daf2bd6bf 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -40,6 +40,10 @@ struct Variable : public Symbol {
         return fModifiers.description() + fType.fName + " " + fName;
     }
 
+    bool dead() const {
+        return !fWriteCount || (!fReadCount && !(fModifiers.fFlags & Modifiers::kOut_Flag));
+    }
+
     mutable Modifiers fModifiers;
     const Type& fType;
     const Storage fStorage;
diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h
index fecb04e2e5..75b7f3dcd4 100644
--- a/src/sksl/ir/SkSLVariableReference.h
+++ b/src/sksl/ir/SkSLVariableReference.h
@@ -38,7 +38,7 @@ struct VariableReference : public Expression {
         }
     }
 
-    virtual ~VariableReference() override {
+    ~VariableReference() override {
         if (fRefKind != kWrite_RefKind) {
             fVariable.fReadCount--;
         }
@@ -64,13 +64,19 @@ struct VariableReference : public Expression {
         fRefKind = refKind;
     }
 
+    bool hasSideEffects() const override {
+        return false;
+    }
+
     SkString description() const override {
         return fVariable.fName;
     }
 
-    virtual std::unique_ptr<Expression> constantPropagate(
-                                                        const IRGenerator& irGenerator,
-                                                        const DefinitionMap& definitions) override {
+    std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
+                                                  const DefinitionMap& definitions) override {
+        if (fRefKind != kRead_RefKind) {
+            return nullptr;
+        }
         auto exprIter = definitions.find(&fVariable);
         if (exprIter != definitions.end() && exprIter->second) {
             const Expression* expr = exprIter->second->get();
@@ -85,6 +91,11 @@ struct VariableReference : public Expression {
                                                                    irGenerator.fContext,
                                                                    Position(),
                                                                    ((FloatLiteral*) expr)->fValue));
+                case Expression::kBoolLiteral_Kind:
+                    return std::unique_ptr<Expression>(new BoolLiteral(
+                                                                    irGenerator.fContext,
+                                                                    Position(),
+                                                                    ((BoolLiteral*) expr)->fValue));
                 default:
                     break;
             }
@@ -93,10 +104,9 @@ struct VariableReference : public Expression {
     }
 
     const Variable& fVariable;
-
-private:
     RefKind fRefKind;
 
+private:
     typedef Expression INHERITED;
 };
 
diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h
index a741a0441d..d4fc5878ae 100644
--- a/src/sksl/ir/SkSLWhileStatement.h
+++ b/src/sksl/ir/SkSLWhileStatement.h
@@ -28,7 +28,7 @@ struct WhileStatement : public Statement {
     }
 
     std::unique_ptr<Expression> fTest;
-    const std::unique_ptr<Statement> fStatement;
+    std::unique_ptr<Statement> fStatement;
 
     typedef Statement INHERITED;
 };