Store TVariable* in TIntermSymbol instead of storing id

src/compiler/translator/IntermNode.cpp

@@ -281,7 +281,7 @@ bool TIntermAggregateBase::insertChildNodes(TIntermSequence::size_type position,
 }
 
 TIntermSymbol::TIntermSymbol(const TVariable *variable)
-    : TIntermTyped(variable->getType()), mId(variable->uniqueId()), mSymbol(variable->name())
+    : TIntermTyped(variable->getType()), mVariable(variable), mSymbol(variable->name())
 {
     if (variable->symbolType() == SymbolType::AngleInternal)
     {
@@ -289,6 +289,11 @@ TIntermSymbol::TIntermSymbol(const TVariable *variable)
     }
 }
 
+const TSymbolUniqueId &TIntermSymbol::uniqueId() const
+{
+    return mVariable->uniqueId();
+}
+
 TIntermAggregate *TIntermAggregate::CreateFunctionCall(const TFunction &func,
                                                        TIntermSequence *arguments)
 {

src/compiler/translator/IntermNode.h

@@ -263,20 +263,20 @@ class TIntermSymbol : public TIntermTyped
 
     bool hasSideEffects() const override { return false; }
 
-    int getId() const { return mId.get(); }
+    const TSymbolUniqueId &uniqueId() const;
     const TString &getSymbol() const { return mSymbol.getString(); }
     const TName &getName() const { return mSymbol; }
+    const TVariable &variable() const { return *mVariable; }
 
     void traverse(TIntermTraverser *it) override;
     TIntermSymbol *getAsSymbolNode() override { return this; }
     bool replaceChildNode(TIntermNode *, TIntermNode *) override { return false; }
 
-  protected:
-    const TSymbolUniqueId mId;
-    TName mSymbol;
-
   private:
     TIntermSymbol(const TIntermSymbol &) = default;  // Note: not deleted, just private!
+
+    const TVariable *const mVariable;  // Guaranteed to be non-null
+    TName mSymbol;
 };
 
 // A Raw node stores raw code, that the translator will insert verbatim

src/compiler/translator/OutputHLSL.cpp

@@ -2408,7 +2408,7 @@ bool OutputHLSL::handleExcessiveLoop(TInfoSinkBase &out, TIntermLoop *node)
     {
         TIntermBinary *test = node->getCondition()->getAsBinaryNode();
 
-        if (test && test->getLeft()->getAsSymbolNode()->getId() == index->getId())
+        if (test && test->getLeft()->getAsSymbolNode()->uniqueId() == index->uniqueId())
         {
             TIntermConstantUnion *constant = test->getRight()->getAsConstantUnion();
 

src/compiler/translator/OutputTree.cpp

@@ -82,7 +82,7 @@ void TOutputTraverser::visitSymbol(TIntermSymbol *node)
     OutputTreeText(mOut, node, mDepth);
 
     mOut << "'" << node->getSymbol() << "' ";
-    mOut << "(symbol id " << node->getId() << ") ";
+    mOut << "(symbol id " << node->uniqueId().get() << ") ";
     mOut << "(" << node->getCompleteString() << ")";
     mOut << "\n";
 }

src/compiler/translator/ParseContext.cpp

@@ -1996,11 +1996,8 @@ bool TParseContext::executeInitializer(const TSourceLoc &line,
         }
         else if (initializer->getAsSymbolNode())
         {
-            const TSymbol *symbol =
-                symbolTable.find(initializer->getAsSymbolNode()->getSymbol(), 0);
-            const TVariable *tVar = static_cast<const TVariable *>(symbol);
-
-            const TConstantUnion *constArray = tVar->getConstPointer();
+            const TVariable &var             = initializer->getAsSymbolNode()->variable();
+            const TConstantUnion *constArray = var.getConstPointer();
             if (constArray)
             {
                 variable->shareConstPointer(constArray);

src/compiler/translator/RemoveUnreferencedVariables.cpp

@@ -92,10 +92,10 @@ void CollectVariableRefCountsTraverser::visitSymbol(TIntermSymbol *node)
 {
     incrementStructTypeRefCount(node->getType());
 
-    auto iter = mSymbolIdRefCounts.find(node->getId());
+    auto iter = mSymbolIdRefCounts.find(node->uniqueId().get());
     if (iter == mSymbolIdRefCounts.end())
     {
-        mSymbolIdRefCounts[node->getId()] = 1u;
+        mSymbolIdRefCounts[node->uniqueId().get()] = 1u;
         return;
     }
     ++(iter->second);
@@ -234,14 +234,14 @@ bool RemoveUnreferencedVariablesTraverser::visitDeclaration(Visit visit, TInterm
         TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
         if (symbolNode != nullptr)
         {
-            canRemoveVariable =
-                (*mSymbolIdRefCounts)[symbolNode->getId()] == 1u || symbolNode->getSymbol().empty();
+            canRemoveVariable = (*mSymbolIdRefCounts)[symbolNode->uniqueId().get()] == 1u ||
+                                symbolNode->getSymbol().empty();
         }
         TIntermBinary *initNode = declarator->getAsBinaryNode();
         if (initNode != nullptr)
         {
             ASSERT(initNode->getLeft()->getAsSymbolNode());
-            int symbolId = initNode->getLeft()->getAsSymbolNode()->getId();
+            int symbolId = initNode->getLeft()->getAsSymbolNode()->uniqueId().get();
             canRemoveVariable =
                 (*mSymbolIdRefCounts)[symbolId] == 1u && !initNode->getRight()->hasSideEffects();
         }
@@ -262,8 +262,8 @@ void RemoveUnreferencedVariablesTraverser::visitSymbol(TIntermSymbol *node)
 {
     if (mRemoveReferences)
     {
-        ASSERT(mSymbolIdRefCounts->find(node->getId()) != mSymbolIdRefCounts->end());
-        --(*mSymbolIdRefCounts)[node->getId()];
+        ASSERT(mSymbolIdRefCounts->find(node->uniqueId().get()) != mSymbolIdRefCounts->end());
+        --(*mSymbolIdRefCounts)[node->uniqueId().get()];
 
         decrementStructTypeRefCount(node->getType());
     }

src/compiler/translator/SymbolUniqueId.cpp

@@ -20,9 +20,17 @@ TSymbolUniqueId::TSymbolUniqueId(const TSymbol &symbol) : mId(symbol.uniqueId().
 {
 }
 
+TSymbolUniqueId::TSymbolUniqueId(const TSymbolUniqueId &) = default;
+TSymbolUniqueId &TSymbolUniqueId::operator=(const TSymbolUniqueId &) = default;
+
 int TSymbolUniqueId::get() const
 {
     return mId;
 }
 
+bool TSymbolUniqueId::operator==(const TSymbolUniqueId &other) const
+{
+    return mId == other.mId;
+}
+
 }  // namespace sh

src/compiler/translator/SymbolUniqueId.h

@@ -22,8 +22,9 @@ class TSymbolUniqueId
     POOL_ALLOCATOR_NEW_DELETE();
     explicit TSymbolUniqueId(TSymbolTable *symbolTable);
     explicit TSymbolUniqueId(const TSymbol &symbol);
-    TSymbolUniqueId(const TSymbolUniqueId &) = default;
-    TSymbolUniqueId &operator=(const TSymbolUniqueId &) = default;
+    TSymbolUniqueId(const TSymbolUniqueId &);
+    TSymbolUniqueId &operator=(const TSymbolUniqueId &);
+    bool operator==(const TSymbolUniqueId &) const;
 
     int get() const;
 

src/compiler/translator/ValidateLimitations.cpp

@@ -25,7 +25,7 @@ int GetLoopSymbolId(TIntermLoop *loop)
     TIntermBinary *declInit  = (*declSeq)[0]->getAsBinaryNode();
     TIntermSymbol *symbol    = declInit->getLeft()->getAsSymbolNode();
 
-    return symbol->getId();
+    return symbol->uniqueId().get();
 }
 
 // Traverses a node to check if it represents a constant index expression.
@@ -55,7 +55,7 @@ class ValidateConstIndexExpr : public TIntermTraverser
         if (mValid)
         {
             bool isLoopSymbol = std::find(mLoopSymbolIds.begin(), mLoopSymbolIds.end(),
-                                          symbol->getId()) != mLoopSymbolIds.end();
+                                          symbol->uniqueId().get()) != mLoopSymbolIds.end();
             mValid = (symbol->getQualifier() == EvqConst) || isLoopSymbol;
         }
     }
@@ -165,7 +165,7 @@ void ValidateLimitationsTraverser::error(TSourceLoc loc, const char *reason, con
 
 bool ValidateLimitationsTraverser::isLoopIndex(TIntermSymbol *symbol)
 {
-    return std::find(mLoopSymbolIds.begin(), mLoopSymbolIds.end(), symbol->getId()) !=
+    return std::find(mLoopSymbolIds.begin(), mLoopSymbolIds.end(), symbol->uniqueId().get()) !=
            mLoopSymbolIds.end();
 }
 
@@ -252,7 +252,7 @@ int ValidateLimitationsTraverser::validateForLoopInit(TIntermLoop *node)
         return -1;
     }
 
-    return symbol->getId();
+    return symbol->uniqueId().get();
 }
 
 bool ValidateLimitationsTraverser::validateForLoopCond(TIntermLoop *node, int indexSymbolId)
@@ -280,7 +280,7 @@ bool ValidateLimitationsTraverser::validateForLoopCond(TIntermLoop *node, int in
         error(binOp->getLine(), "Invalid condition", "for");
         return false;
     }
-    if (symbol->getId() != indexSymbolId)
+    if (symbol->uniqueId().get() != indexSymbolId)
     {
         error(symbol->getLine(), "Expected loop index", symbol->getSymbol().c_str());
         return false;
@@ -351,7 +351,7 @@ bool ValidateLimitationsTraverser::validateForLoopExpr(TIntermLoop *node, int in
         error(expr->getLine(), "Invalid expression", "for");
         return false;
     }
-    if (symbol->getId() != indexSymbolId)
+    if (symbol->uniqueId().get() != indexSymbolId)
     {
         error(symbol->getLine(), "Expected loop index", symbol->getSymbol().c_str());
         return false;

src/tests/compiler_tests/IntermNode_test.cpp

@@ -74,7 +74,8 @@ class IntermNodeTest : public testing::Test
         ASSERT_NE(nullptr, copy);
         ASSERT_NE(nullptr, original);
         ASSERT_NE(original, copy);
-        ASSERT_EQ(original->getId(), copy->getId());
+        ASSERT_EQ(&original->variable(), &copy->variable());
+        ASSERT_EQ(original->uniqueId(), copy->uniqueId());
         ASSERT_EQ(original->getName().getString(), copy->getName().getString());
         ASSERT_EQ(original->getName().isInternal(), copy->getName().isInternal());
         checkTypeEqualWithQualifiers(original->getType(), copy->getType());