Always create TVariables for TIntermSymbol nodes

TIntermSymbol nodes are now constructed based on a specific TVariable.
This makes sure that all TIntermSymbol nodes that are created to refer
to a specific temporary in an AST transform will have consistent data.
The TVariable objects are not necessarily added to the symbol table
levels - just those variables that can be referred to by their name
during parsing need to be reachable through there.

In the future this can be taken a step further so that TIntermSymbol
nodes just to point to a TVariable instead of duplicating the
information.

BUG=angleproject:2267
TEST=angle_unittests

Change-Id: I4e7bcdb0637cd3b588d3c202ef02f4b7bd7954a1
Reviewed-on: https://chromium-review.googlesource.com/811925
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 5e36b86..c6bd784 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -1106,6 +1106,8 @@
 {
     ASSERT((*variable) == nullptr);
 
+    (*variable) = new TVariable(&symbolTable, &identifier, type, SymbolType::UserDefined);
+
     checkBindingIsValid(line, type);
 
     bool needsReservedCheck = true;
@@ -1140,8 +1142,7 @@
     if (needsReservedCheck && !checkIsNotReserved(line, identifier))
         return false;
 
-    (*variable) = symbolTable.declareVariable(&identifier, type);
-    if (!(*variable))
+    if (!symbolTable.declareVariable(*variable))
     {
         error(line, "redefinition", identifier.c_str());
         return false;
@@ -1889,12 +1890,12 @@
     {
         ASSERT(mGeometryShaderInputArraySize > 0u);
 
-        node = new TIntermSymbol(variable->uniqueId(), variable->name(), variableType);
+        node = new TIntermSymbol(variable);
         node->getTypePointer()->sizeOutermostUnsizedArray(mGeometryShaderInputArraySize);
     }
     else
     {
-        node = new TIntermSymbol(variable->uniqueId(), variable->name(), variableType);
+        node = new TIntermSymbol(variable);
     }
     ASSERT(node != nullptr);
     node->setLine(location);
@@ -1914,7 +1915,6 @@
     ASSERT(initNode != nullptr);
     ASSERT(*initNode == nullptr);
 
-    TVariable *variable = nullptr;
     if (type.isUnsizedArray())
     {
         // In case initializer is not an array or type has more dimensions than initializer, this
@@ -1924,6 +1924,8 @@
         auto *arraySizes = initializer->getType().getArraySizes();
         type.sizeUnsizedArrays(arraySizes);
     }
+
+    TVariable *variable = nullptr;
     if (!declareVariable(line, identifier, type, &variable))
     {
         return false;
@@ -2008,8 +2010,7 @@
         }
     }
 
-    TIntermSymbol *intermSymbol =
-        new TIntermSymbol(variable->uniqueId(), variable->name(), variable->getType());
+    TIntermSymbol *intermSymbol = new TIntermSymbol(variable);
     intermSymbol->setLine(line);
     *initNode = createAssign(EOpInitialize, intermSymbol, initializer, line);
     if (*initNode == nullptr)
@@ -2443,7 +2444,9 @@
         // But if the empty declaration is declaring a struct type, the symbol node will store that.
         if (type.getBasicType() == EbtStruct)
         {
-            symbol = new TIntermSymbol(symbolTable.getEmptySymbolId(), "", type);
+            TVariable *emptyVariable =
+                new TVariable(&symbolTable, NewPoolTString(""), type, SymbolType::Empty);
+            symbol = new TIntermSymbol(emptyVariable);
         }
         else if (IsAtomicCounter(publicType.getBasicType()))
         {
@@ -2459,11 +2462,9 @@
         checkAtomicCounterOffsetDoesNotOverlap(false, identifierOrTypeLocation, &type);
 
         TVariable *variable = nullptr;
-        declareVariable(identifierOrTypeLocation, identifier, type, &variable);
-
-        if (variable)
+        if (declareVariable(identifierOrTypeLocation, identifier, type, &variable))
         {
-            symbol = new TIntermSymbol(variable->uniqueId(), identifier, type);
+            symbol = new TIntermSymbol(variable);
         }
     }
 
@@ -2502,15 +2503,13 @@
 
     checkAtomicCounterOffsetDoesNotOverlap(false, identifierLocation, &arrayType);
 
-    TVariable *variable = nullptr;
-    declareVariable(identifierLocation, identifier, arrayType, &variable);
-
     TIntermDeclaration *declaration = new TIntermDeclaration();
     declaration->setLine(identifierLocation);
 
-    if (variable)
+    TVariable *variable = nullptr;
+    if (declareVariable(identifierLocation, identifier, arrayType, &variable))
     {
-        TIntermSymbol *symbol = new TIntermSymbol(variable->uniqueId(), identifier, arrayType);
+        TIntermSymbol *symbol = new TIntermSymbol(variable);
         symbol->setLine(identifierLocation);
         declaration->appendDeclarator(symbol);
     }
@@ -2633,7 +2632,7 @@
 
     symbolTable.addInvariantVarying(std::string(identifier->c_str()));
 
-    TIntermSymbol *intermSymbol = new TIntermSymbol(variable->uniqueId(), *identifier, type);
+    TIntermSymbol *intermSymbol = new TIntermSymbol(variable);
     intermSymbol->setLine(identifierLoc);
 
     return new TIntermInvariantDeclaration(intermSymbol, identifierLoc);
@@ -2654,7 +2653,6 @@
 
     checkDeclaratorLocationIsNotSpecified(identifierLocation, publicType);
 
-    TVariable *variable = nullptr;
     TType type(publicType);
 
     checkGeometryShaderInputAndSetArraySize(identifierLocation, identifier.c_str(), &type);
@@ -2663,11 +2661,10 @@
 
     checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, &type);
 
-    declareVariable(identifierLocation, identifier, type, &variable);
-
-    if (variable)
+    TVariable *variable = nullptr;
+    if (declareVariable(identifierLocation, identifier, type, &variable))
     {
-        TIntermSymbol *symbol = new TIntermSymbol(variable->uniqueId(), identifier, type);
+        TIntermSymbol *symbol = new TIntermSymbol(variable);
         symbol->setLine(identifierLocation);
         declarationOut->appendDeclarator(symbol);
     }
@@ -2702,11 +2699,9 @@
         checkAtomicCounterOffsetDoesNotOverlap(true, identifierLocation, &arrayType);
 
         TVariable *variable = nullptr;
-        declareVariable(identifierLocation, identifier, arrayType, &variable);
-
-        if (variable)
+        if (declareVariable(identifierLocation, identifier, arrayType, &variable))
         {
-            TIntermSymbol *symbol = new TIntermSymbol(variable->uniqueId(), identifier, arrayType);
+            TIntermSymbol *symbol = new TIntermSymbol(variable);
             symbol->setLine(identifierLocation);
             declarationOut->appendDeclarator(symbol);
         }
@@ -3177,16 +3172,13 @@
         // be used for unused args).
         if (param.name != nullptr)
         {
+            TVariable *variable =
+                new TVariable(&symbolTable, param.name, *param.type, SymbolType::UserDefined);
+            symbol = new TIntermSymbol(variable);
             // Insert the parameter in the symbol table.
             if (insertParametersToSymbolTable)
             {
-                TVariable *variable = symbolTable.declareVariable(param.name, *param.type);
-                if (variable)
-                {
-                    symbol = new TIntermSymbol(variable->uniqueId(), variable->name(),
-                                               variable->getType());
-                }
-                else
+                if (!symbolTable.declareVariable(variable))
                 {
                     error(location, "redefinition", param.name->c_str());
                 }
@@ -3207,7 +3199,9 @@
         {
             // The parameter had no name or declaring the symbol failed - either way, add a nameless
             // symbol.
-            symbol = new TIntermSymbol(symbolTable.getEmptySymbolId(), "", *param.type);
+            TVariable *emptyVariable =
+                new TVariable(&symbolTable, NewPoolTString(""), *param.type, SymbolType::Empty);
+            symbol = new TIntermSymbol(emptyVariable);
         }
         symbol->setLine(location);
         prototype->appendParameter(symbol);
@@ -3480,7 +3474,7 @@
         type->setBasicType(EbtFloat);
     }
 
-    return new TFunction(&symbolTable, nullptr, type, SymbolType::BuiltIn, EOpConstruct);
+    return new TFunction(&symbolTable, nullptr, type, SymbolType::NotResolved, EOpConstruct);
 }
 
 void TParseContext::checkIsNotUnsizedArray(const TSourceLoc &line,
@@ -3819,11 +3813,18 @@
         interfaceBlockType.makeArray(arraySize);
     }
 
-    TString symbolName = "";
-    const TSymbolUniqueId *symbolId = nullptr;
-
+    // The instance variable gets created to refer to the interface block type from the AST
+    // regardless of if there's an instance name. It just has an empty name if there's no instance
+    // name.
     if (!instanceName)
     {
+        instanceName = NewPoolTString("");
+    }
+    TVariable *instanceVariable =
+        new TVariable(&symbolTable, instanceName, interfaceBlockType, SymbolType::UserDefined);
+
+    if (instanceName->empty())
+    {
         // define symbols for the members of the interface block
         for (size_t memberIndex = 0; memberIndex < fieldList->size(); ++memberIndex)
         {
@@ -3833,9 +3834,9 @@
             // set parent pointer of the field variable
             fieldType->setInterfaceBlock(interfaceBlock);
 
-            TVariable *fieldVariable = symbolTable.declareVariable(&field->name(), *fieldType);
-
-            if (fieldVariable)
+            TVariable *fieldVariable =
+                new TVariable(&symbolTable, &field->name(), *fieldType, SymbolType::UserDefined);
+            if (symbolTable.declareVariable(fieldVariable))
             {
                 fieldVariable->setQualifier(typeQualifier.qualifier);
             }
@@ -3845,37 +3846,24 @@
                       field->name().c_str());
             }
         }
-        symbolId = &symbolTable.getEmptySymbolId();
     }
     else
     {
         checkIsNotReserved(instanceLine, *instanceName);
 
         // add a symbol for this interface block
-        TVariable *instanceTypeDef = symbolTable.declareVariable(instanceName, interfaceBlockType);
-        if (instanceTypeDef)
-        {
-            instanceTypeDef->setQualifier(typeQualifier.qualifier);
-            symbolId = &instanceTypeDef->uniqueId();
-        }
-        else
+        if (!symbolTable.declareVariable(instanceVariable))
         {
             error(instanceLine, "redefinition of an interface block instance name",
                   instanceName->c_str());
         }
-        symbolName = *instanceName;
     }
 
-    TIntermDeclaration *declaration = nullptr;
-
-    if (symbolId)
-    {
-        TIntermSymbol *blockSymbol = new TIntermSymbol(*symbolId, symbolName, interfaceBlockType);
-        blockSymbol->setLine(typeQualifier.line);
-        declaration = new TIntermDeclaration();
-        declaration->appendDeclarator(blockSymbol);
-        declaration->setLine(nameLine);
-    }
+    TIntermSymbol *blockSymbol = new TIntermSymbol(instanceVariable);
+    blockSymbol->setLine(typeQualifier.line);
+    TIntermDeclaration *declaration = new TIntermDeclaration();
+    declaration->appendDeclarator(blockSymbol);
+    declaration->setLine(nameLine);
 
     exitStructDeclaration();
     return declaration;