Make AST path always include the current node being traversed

src/compiler/translator/IntermNode.h

@@ -962,6 +962,7 @@ class TIntermTraverser : angle::NonCopyable
     void useTemporaryIndex(unsigned int *temporaryIndex);
 
   protected:
+    // Should only be called from traverse*() functions
     void incrementDepth(TIntermNode *current)
     {
         mDepth++;
@@ -969,20 +970,35 @@ class TIntermTraverser : angle::NonCopyable
         mPath.push_back(current);
     }
 
+    // Should only be called from traverse*() functions
     void decrementDepth()
     {
         mDepth--;
         mPath.pop_back();
     }
 
-    TIntermNode *getParentNode() { return mPath.size() == 0 ? NULL : mPath.back(); }
+    // RAII helper for incrementDepth/decrementDepth
+    class ScopedNodeInTraversalPath
+    {
+      public:
+        ScopedNodeInTraversalPath(TIntermTraverser *traverser, TIntermNode *current)
+            : mTraverser(traverser)
+        {
+            mTraverser->incrementDepth(current);
+        }
+        ~ScopedNodeInTraversalPath() { mTraverser->decrementDepth(); }
+      private:
+        TIntermTraverser *mTraverser;
+    };
+
+    TIntermNode *getParentNode() { return mPath.size() <= 1 ? nullptr : mPath[mPath.size() - 2u]; }
 
     // Return the nth ancestor of the node being traversed. getAncestorNode(0) == getParentNode()
     TIntermNode *getAncestorNode(unsigned int n)
     {
-        if (mPath.size() > n)
+        if (mPath.size() > n + 1u)
         {
-            return mPath[mPath.size() - n - 1u];
+            return mPath[mPath.size() - n - 2u];
         }
         return nullptr;
     }
@@ -991,11 +1007,6 @@ class TIntermTraverser : angle::NonCopyable
     void incrementParentBlockPos();
     void popParentBlock();
 
-    bool parentNodeIsBlock()
-    {
-        return !mParentBlockStack.empty() && getParentNode() == mParentBlockStack.back().node;
-    }
-
     // To replace a single node with multiple nodes on the parent aggregate node
     struct NodeReplaceWithMultipleEntry
     {
@@ -1086,9 +1097,6 @@ class TIntermTraverser : angle::NonCopyable
     int mDepth;
     int mMaxDepth;
 
-    // All the nodes from root to the current node's parent during traversing.
-    TVector<TIntermNode *> mPath;
-
     bool mInGlobalScope;
 
     // During traversing, save all the changes that need to happen into
@@ -1131,6 +1139,9 @@ class TIntermTraverser : angle::NonCopyable
 
     std::vector<NodeUpdateEntry> mReplacements;
 
+    // All the nodes from root to the current node during traversing.
+    TVector<TIntermNode *> mPath;
+
     // All the code blocks from the root to the current node's parent during traversal.
     std::vector<ParentBlock> mParentBlockStack;
 

src/compiler/translator/OutputGLSLBase.cpp

@@ -761,7 +761,6 @@ bool TOutputGLSLBase::visitIfElse(Visit visit, TIntermIfElse *node)
     node->getCondition()->traverse(this);
     out << ")\n";
 
-    incrementDepth(node);
     visitCodeBlock(node->getTrueBlock());
 
     if (node->getFalseBlock())
@@ -769,7 +768,6 @@ bool TOutputGLSLBase::visitIfElse(Visit visit, TIntermIfElse *node)
         out << "else\n";
         visitCodeBlock(node->getFalseBlock());
     }
-    decrementDepth();
     return false;
 }
 
@@ -812,7 +810,6 @@ bool TOutputGLSLBase::visitBlock(Visit visit, TIntermBlock *node)
         out << "{\n";
     }
 
-    incrementDepth(node);
     for (TIntermSequence::const_iterator iter = node->getSequence()->begin();
          iter != node->getSequence()->end(); ++iter)
     {
@@ -823,7 +820,6 @@ bool TOutputGLSLBase::visitBlock(Visit visit, TIntermBlock *node)
         if (isSingleStatement(curNode))
             out << ";\n";
     }
-    decrementDepth();
 
     // Scope the blocks except when at the global scope.
     if (mDepth > 0)
@@ -835,11 +831,9 @@ bool TOutputGLSLBase::visitBlock(Visit visit, TIntermBlock *node)
 
 bool TOutputGLSLBase::visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node)
 {
-    incrementDepth(node);
     TIntermFunctionPrototype *prototype = node->getFunctionPrototype();
     prototype->traverse(this);
     visitCodeBlock(node->getBody());
-    decrementDepth();
 
     // Fully processed; no need to visit children.
     return false;
@@ -986,8 +980,6 @@ bool TOutputGLSLBase::visitLoop(Visit visit, TIntermLoop *node)
 {
     TInfoSinkBase &out = objSink();
 
-    incrementDepth(node);
-
     TLoopType loopType = node->getType();
 
     if (loopType == ELoopFor)  // for loop
@@ -1029,8 +1021,6 @@ bool TOutputGLSLBase::visitLoop(Visit visit, TIntermLoop *node)
         out << ");\n";
     }
 
-    decrementDepth();
-
     // No need to visit children. They have been already processed in
     // this function.
     return false;

src/compiler/translator/OutputHLSL.cpp

@@ -919,11 +919,9 @@ void OutputHLSL::outputEqual(Visit visit, const TType &type, TOperator op, TInfo
     }
 }
 
-bool OutputHLSL::ancestorEvaluatesToSamplerInStruct(Visit visit)
+bool OutputHLSL::ancestorEvaluatesToSamplerInStruct()
 {
-    // Inside InVisit the current node is already in the path.
-    const unsigned int initialN = visit == InVisit ? 1u : 0u;
-    for (unsigned int n = initialN; getAncestorNode(n) != nullptr; ++n)
+    for (unsigned int n = 0u; getAncestorNode(n) != nullptr; ++n)
     {
         TIntermNode *ancestor               = getAncestorNode(n);
         const TIntermBinary *ancestorBinary = ancestor->getAsBinaryNode();
@@ -1126,7 +1124,7 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
                     return false;
                 }
             }
-            else if (ancestorEvaluatesToSamplerInStruct(visit))
+            else if (ancestorEvaluatesToSamplerInStruct())
             {
                 // All parts of an expression that access a sampler in a struct need to use _ as
                 // separator to access the sampler variable that has been moved out of the struct.
@@ -1163,7 +1161,7 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
             {
                 // All parts of an expression that access a sampler in a struct need to use _ as
                 // separator to access the sampler variable that has been moved out of the struct.
-                indexingReturnsSampler = ancestorEvaluatesToSamplerInStruct(visit);
+                indexingReturnsSampler = ancestorEvaluatesToSamplerInStruct();
             }
             if (visit == InVisit)
             {

src/compiler/translator/OutputHLSL.h

@@ -239,7 +239,7 @@ class OutputHLSL : public TIntermTraverser
 
   private:
     TString samplerNamePrefixFromStruct(TIntermTyped *node);
-    bool ancestorEvaluatesToSamplerInStruct(Visit visit);
+    bool ancestorEvaluatesToSamplerInStruct();
 };
 }
 

src/compiler/translator/SimplifyLoopConditions.cpp

@@ -135,9 +135,7 @@ void SimplifyLoopConditionsTraverser::traverseLoop(TIntermLoop *node)
 
     // Mark that we're inside a loop condition or expression, and transform the loop if needed.
 
-    incrementDepth(node);
-
-    // Note: No need to traverse the loop init node.
+    ScopedNodeInTraversalPath addToPath(this, node);
 
     mInsideLoopInitConditionOrExpression = true;
     TLoopType loopType                   = node->getType();
@@ -274,8 +272,7 @@ void SimplifyLoopConditionsTraverser::traverseLoop(TIntermLoop *node)
                 ELoopWhile, nullptr, createTempSymbol(conditionInitializer->getType()), nullptr,
                 whileLoopBody);
             loopScope->getSequence()->push_back(whileLoop);
-            queueReplacementWithParent(getAncestorNode(1), node, loopScope,
-                                       OriginalNode::IS_DROPPED);
+            queueReplacement(node, loopScope, OriginalNode::IS_DROPPED);
         }
     }
 
@@ -283,8 +280,6 @@ void SimplifyLoopConditionsTraverser::traverseLoop(TIntermLoop *node)
 
     if (!mFoundLoopToChange && node->getBody())
         node->getBody()->traverse(this);
-
-    decrementDepth();
 }
 
 }  // namespace

src/tests/compiler_tests/WEBGL_multiview_test.cpp

@@ -381,3 +381,19 @@ TEST_F(WEBGLMultiviewVertexShaderTest, ValidUseOfExtensionMacros)
         FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog;
     }
 }
+
+// Test that the parent node is tracked correctly when validating assignment to gl_Position.
+TEST_F(WEBGLMultiviewVertexShaderTest, AssignmentWithViewIDInsideAssignment)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "#extension GL_OVR_multiview : require\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position.y = (gl_Position.x = (gl_ViewID_OVR == 0u) ? 1.0 : 0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
+    }
+}