Refactor l-value tracking to a separate traverser parent class

src/compiler/translator/EmulatePrecision.cpp

@@ -292,7 +292,7 @@ bool parentUsesResult(TIntermNode* parent, TIntermNode* node)
 }  // namespace anonymous
 
 EmulatePrecision::EmulatePrecision()
-    : TIntermTraverser(true, true, true), mDeclaringVariables(false)
+    : TLValueTrackingTraverser(true, true, true), mDeclaringVariables(false)
 {}
 
 void EmulatePrecision::visitSymbol(TIntermSymbol *node)

src/compiler/translator/EmulatePrecision.h

@@ -18,7 +18,7 @@
 // need to write a huge number of variations of the emulated compound assignment
 // to every translated shader with emulation enabled.
 
-class EmulatePrecision : public TIntermTraverser
+class EmulatePrecision : public TLValueTrackingTraverser
 {
   public:
     EmulatePrecision();

src/compiler/translator/IntermNode.h

@@ -613,9 +613,7 @@ class TIntermTraverser : angle::NonCopyable
           postVisit(postVisit),
           mDepth(0),
           mMaxDepth(0),
-          mTemporaryIndex(nullptr),
-          mOperatorRequiresLValue(false),
-          mInFunctionCallOutParameter(false)
+          mTemporaryIndex(nullptr)
     {
     }
     virtual ~TIntermTraverser() {}
@@ -693,36 +691,6 @@ class TIntermTraverser : angle::NonCopyable
     const bool inVisit;
     const bool postVisit;
 
-    // Track whether an l-value is required in the node that is currently being traversed.
-    // These functions are intended to be called only from traversal functions, not from subclasses
-    // of TIntermTraverser.
-    // Use isLValueRequiredHere instead to check all conditions which require an l-value.
-    void setOperatorRequiresLValue(bool lValueRequired)
-    {
-        mOperatorRequiresLValue = lValueRequired;
-    }
-    bool operatorRequiresLValue() const { return mOperatorRequiresLValue; }
-
-    // Add a function encountered during traversal to the function map. Intended to be called only
-    // from traversal functions, not from subclasses of TIntermTraverser.
-    void addToFunctionMap(const TString &name, TIntermSequence *paramSequence);
-
-    // Return true if the prototype or definition of the function being called has been encountered
-    // during traversal.
-    bool isInFunctionMap(const TIntermAggregate *callNode) const;
-    // Return the parameters sequence from the function definition or prototype.
-    TIntermSequence *getFunctionParameters(const TIntermAggregate *callNode);
-
-    // Track whether an l-value is required inside a function call.
-    // These functions are intended to be called only from traversal functions, not from traverers.
-    void setInFunctionCallOutParameter(bool inOutParameter);
-    bool isInFunctionCallOutParameter() const;
-
-    bool isLValueRequiredHere() const
-    {
-        return mOperatorRequiresLValue || mInFunctionCallOutParameter;
-    }
-
     int mDepth;
     int mMaxDepth;
 
@@ -822,6 +790,54 @@ class TIntermTraverser : angle::NonCopyable
     std::vector<ParentBlock> mParentBlockStack;
 
     unsigned int *mTemporaryIndex;
+};
+
+// Traverser parent class that tracks where a node is a destination of a write operation and so is
+// required to be an l-value.
+class TLValueTrackingTraverser : public TIntermTraverser
+{
+  public:
+    TLValueTrackingTraverser(bool preVisit, bool inVisit, bool postVisit)
+        : TIntermTraverser(preVisit, inVisit, postVisit),
+          mOperatorRequiresLValue(false),
+          mInFunctionCallOutParameter(false)
+    {
+    }
+    virtual ~TLValueTrackingTraverser() {}
+
+    void traverseBinary(TIntermBinary *node) override;
+    void traverseUnary(TIntermUnary *node) override;
+    void traverseAggregate(TIntermAggregate *node) override;
+
+  protected:
+    bool isLValueRequiredHere() const
+    {
+        return mOperatorRequiresLValue || mInFunctionCallOutParameter;
+    }
+
+    // Return true if the prototype or definition of the function being called has been encountered
+    // during traversal.
+    bool isInFunctionMap(const TIntermAggregate *callNode) const;
+
+  private:
+    // Track whether an l-value is required in the node that is currently being traversed by the
+    // surrounding operator.
+    // Use isLValueRequiredHere to check all conditions which require an l-value.
+    void setOperatorRequiresLValue(bool lValueRequired)
+    {
+        mOperatorRequiresLValue = lValueRequired;
+    }
+    bool operatorRequiresLValue() const { return mOperatorRequiresLValue; }
+
+    // Add a function encountered during traversal to the function map.
+    void addToFunctionMap(const TString &name, TIntermSequence *paramSequence);
+
+    // Return the parameters sequence from the function definition or prototype.
+    TIntermSequence *getFunctionParameters(const TIntermAggregate *callNode);
+
+    // Track whether an l-value is required inside a function call.
+    void setInFunctionCallOutParameter(bool inOutParameter);
+    bool isInFunctionCallOutParameter() const;
 
     bool mOperatorRequiresLValue;
     bool mInFunctionCallOutParameter;

src/compiler/translator/IntermTraverse.cpp

@@ -151,29 +151,29 @@ void TIntermTraverser::nextTemporaryIndex()
     ++(*mTemporaryIndex);
 }
 
-void TIntermTraverser::addToFunctionMap(const TString &name, TIntermSequence *paramSequence)
+void TLValueTrackingTraverser::addToFunctionMap(const TString &name, TIntermSequence *paramSequence)
 {
     mFunctionMap[name] = paramSequence;
 }
 
-bool TIntermTraverser::isInFunctionMap(const TIntermAggregate *callNode) const
+bool TLValueTrackingTraverser::isInFunctionMap(const TIntermAggregate *callNode) const
 {
     ASSERT(callNode->getOp() == EOpFunctionCall || callNode->getOp() == EOpInternalFunctionCall);
     return (mFunctionMap.find(callNode->getName()) != mFunctionMap.end());
 }
 
-TIntermSequence *TIntermTraverser::getFunctionParameters(const TIntermAggregate *callNode)
+TIntermSequence *TLValueTrackingTraverser::getFunctionParameters(const TIntermAggregate *callNode)
 {
     ASSERT(isInFunctionMap(callNode));
     return mFunctionMap[callNode->getName()];
 }
 
-void TIntermTraverser::setInFunctionCallOutParameter(bool inOutParameter)
+void TLValueTrackingTraverser::setInFunctionCallOutParameter(bool inOutParameter)
 {
     mInFunctionCallOutParameter = inOutParameter;
 }
 
-bool TIntermTraverser::isInFunctionCallOutParameter() const
+bool TLValueTrackingTraverser::isInFunctionCallOutParameter() const
 {
     return mInFunctionCallOutParameter;
 }
@@ -222,6 +222,43 @@ void TIntermTraverser::traverseBinary(TIntermBinary *node)
     {
         incrementDepth(node);
 
+        if (node->getLeft())
+            node->getLeft()->traverse(this);
+
+        if (inVisit)
+            visit = visitBinary(InVisit, node);
+
+        if (visit && node->getRight())
+            node->getRight()->traverse(this);
+
+        decrementDepth();
+    }
+
+    //
+    // Visit the node after the children, if requested and the traversal
+    // hasn't been cancelled yet.
+    //
+    if (visit && postVisit)
+        visitBinary(PostVisit, node);
+}
+
+void TLValueTrackingTraverser::traverseBinary(TIntermBinary *node)
+{
+    bool visit = true;
+
+    //
+    // visit the node before children if pre-visiting.
+    //
+    if (preVisit)
+        visit = visitBinary(PreVisit, node);
+
+    //
+    // Visit the children, in the right order.
+    //
+    if (visit)
+    {
+        incrementDepth(node);
+
         // Some binary operations like indexing can be inside an expression which must be an
         // l-value.
         bool parentOperatorRequiresLValue     = operatorRequiresLValue();
@@ -282,6 +319,26 @@ void TIntermTraverser::traverseUnary(TIntermUnary *node)
     {
         incrementDepth(node);
 
+        node->getOperand()->traverse(this);
+
+        decrementDepth();
+    }
+
+    if (visit && postVisit)
+        visitUnary(PostVisit, node);
+}
+
+void TLValueTrackingTraverser::traverseUnary(TIntermUnary *node)
+{
+    bool visit = true;
+
+    if (preVisit)
+        visit = visitUnary(PreVisit, node);
+
+    if (visit)
+    {
+        incrementDepth(node);
+
         ASSERT(!operatorRequiresLValue());
         switch (node->getOp())
         {
@@ -314,6 +371,45 @@ void TIntermTraverser::traverseAggregate(TIntermAggregate *node)
     bool visit = true;
 
     TIntermSequence *sequence = node->getSequence();
+
+    if (preVisit)
+        visit = visitAggregate(PreVisit, node);
+
+    if (visit)
+    {
+        incrementDepth(node);
+
+        if (node->getOp() == EOpSequence)
+            pushParentBlock(node);
+
+        for (auto *child : *sequence)
+        {
+            child->traverse(this);
+            if (visit && inVisit)
+            {
+                if (child != sequence->back())
+                    visit = visitAggregate(InVisit, node);
+            }
+
+            if (node->getOp() == EOpSequence)
+                incrementParentBlockPos();
+        }
+
+        if (node->getOp() == EOpSequence)
+            popParentBlock();
+
+        decrementDepth();
+    }
+
+    if (visit && postVisit)
+        visitAggregate(PostVisit, node);
+}
+
+void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
+{
+    bool visit = true;
+
+    TIntermSequence *sequence = node->getSequence();
     switch (node->getOp())
     {
         case EOpFunction: