Fix issues with the conditional discard workarounds to do with assignments.

The old modifiesState method really checked if an operator was an assignment,
so restored that behaviour and use the new side effects detection only for
the new code.

ANGLEBUG=486
BUG=
R=nicolascapens@chromium.org, zmo@chromium.org

Review URL: https://codereview.appspot.com/22130043

4 files changed, 8 insertions, 8 deletions

diff --git a/src/compiler/Intermediate.cpp b/src/compiler/Intermediate.cpp
index b3cb3ddb..ef858215 100644
--- a/src/compiler/Intermediate.cpp
+++ b/src/compiler/Intermediate.cpp
@@ -804,9 +804,7 @@ bool TIntermSelection::replaceChildNode(
 //
 // Say whether or not an operation node changes the value of a variable.
 //
-// Returns true if state is modified.
-//
-bool TIntermOperator::hasSideEffects() const
+bool TIntermOperator::isAssignment() const
 {
     switch (op) {
         case EOpPostIncrement:
diff --git a/src/compiler/ValidateLimitations.cpp b/src/compiler/ValidateLimitations.cpp
index 3f3260b0..64969c48 100644
--- a/src/compiler/ValidateLimitations.cpp
+++ b/src/compiler/ValidateLimitations.cpp
@@ -457,7 +457,7 @@ bool ValidateLimitations::validateFunctionCall(TIntermAggregate* node)
 bool ValidateLimitations::validateOperation(TIntermOperator* node,
                                             TIntermNode* operand) {
     // Check if loop index is modified in the loop body.
-    if (!withinLoopBody() || !node->hasSideEffects())
+    if (!withinLoopBody() || !node->isAssignment())
         return true;
 
     const TIntermSymbol* symbol = operand->getAsSymbolNode();
diff --git a/src/compiler/depgraph/DependencyGraphBuilder.cpp b/src/compiler/depgraph/DependencyGraphBuilder.cpp
index 069e9633..026e6d57 100644
--- a/src/compiler/depgraph/DependencyGraphBuilder.cpp
+++ b/src/compiler/depgraph/DependencyGraphBuilder.cpp
@@ -94,7 +94,7 @@ void TDependencyGraphBuilder::visitSymbol(TIntermSymbol* intermSymbol)
 bool TDependencyGraphBuilder::visitBinary(Visit visit, TIntermBinary* intermBinary)
 {
     TOperator op = intermBinary->getOp();
-    if (op == EOpInitialize || intermBinary->hasSideEffects())
+    if (op == EOpInitialize || intermBinary->isAssignment())
         visitAssignment(intermBinary);
     else if (op == EOpLogicalAnd || op == EOpLogicalOr)
         visitLogicalOp(intermBinary);
diff --git a/src/compiler/intermediate.h b/src/compiler/intermediate.h
index 4ddfdabd..14e39fd5 100644
--- a/src/compiler/intermediate.h
+++ b/src/compiler/intermediate.h
@@ -406,9 +406,11 @@ public:
     TOperator getOp() const { return op; }
     void setOp(TOperator o) { op = o; }
 
-    virtual bool hasSideEffects() const;
+    bool isAssignment() const;
     bool isConstructor() const;
 
+    virtual bool hasSideEffects() const { return isAssignment(); }
+
 protected:
     TIntermOperator(TOperator o) : TIntermTyped(TType(EbtFloat, EbpUndefined)), op(o) {}
     TIntermOperator(TOperator o, TType& t) : TIntermTyped(t), op(o) {}   
@@ -427,7 +429,7 @@ public:
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
-    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || left->hasSideEffects() || right->hasSideEffects()); }
+    virtual bool hasSideEffects() const { return (isAssignment() || left->hasSideEffects() || right->hasSideEffects()); }
 
     void setLeft(TIntermTyped* n) { left = n; }
     void setRight(TIntermTyped* n) { right = n; }
@@ -459,7 +461,7 @@ public:
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
-    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || operand->hasSideEffects()); }
+    virtual bool hasSideEffects() const { return (isAssignment() || operand->hasSideEffects()); }
 
     void setOperand(TIntermTyped* o) { operand = o; }
     TIntermTyped* getOperand() { return operand; }