Clean up inserting variables to symbol table

This makes the TSymbolTable interface cleaner and prepares for making
unique id counting thread-safe.

BUG=angleproject:624
TEST=angle_unittests

Change-Id: Ief99c9fc777603de28ba1517e351bc8a00633590
Reviewed-on: https://chromium-review.googlesource.com/570418
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/Initialize.cpp b/src/compiler/translator/Initialize.cpp
index 4275837..2f93307 100644
--- a/src/compiler/translator/Initialize.cpp
+++ b/src/compiler/translator/Initialize.cpp
@@ -690,15 +690,10 @@
     fields->push_back(diff);
     TStructure *depthRangeStruct =
         new TStructure(NewPoolTString("gl_DepthRangeParameters"), fields);
-    TVariable *depthRangeParameters =
-        new TVariable(&depthRangeStruct->name(), TType(depthRangeStruct), true);
-    symbolTable.insert(COMMON_BUILTINS, depthRangeParameters);
-    TVariable *depthRange = new TVariable(NewPoolTString("gl_DepthRange"), TType(depthRangeStruct));
-    depthRange->setQualifier(EvqUniform);
-    // Do lazy initialization for depth range, so we allocate to the current scope.
-    depthRangeParameters->getType().realize();
-    depthRange->getType().realize();
-    symbolTable.insert(COMMON_BUILTINS, depthRange);
+    symbolTable.insertStructType(COMMON_BUILTINS, depthRangeStruct);
+    TType depthRangeType(depthRangeStruct);
+    depthRangeType.setQualifier(EvqUniform);
+    symbolTable.insertVariable(COMMON_BUILTINS, "gl_DepthRange", depthRangeType);
 
     //
     // Implementation dependent built-in constants.
@@ -796,34 +791,28 @@
 
     if (resources.OVR_multiview && type != GL_COMPUTE_SHADER)
     {
-        symbolTable.insert(ESSL3_BUILTINS, "GL_OVR_multiview",
-                           new TVariable(NewPoolTString("gl_ViewID_OVR"),
-                                         TType(EbtUInt, EbpHigh, EvqViewIDOVR, 1)));
+        symbolTable.insertVariableExt(ESSL3_BUILTINS, "GL_OVR_multiview", "gl_ViewID_OVR",
+                                      TType(EbtUInt, EbpHigh, EvqViewIDOVR, 1));
 
         // ESSL 1.00 doesn't have unsigned integers, so gl_ViewID_OVR is a signed integer in ESSL
         // 1.00. This is specified in the WEBGL_multiview spec.
-        symbolTable.insert(ESSL1_BUILTINS, "GL_OVR_multiview",
-                           new TVariable(NewPoolTString("gl_ViewID_OVR"),
-                                         TType(EbtInt, EbpHigh, EvqViewIDOVR, 1)));
+        symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_OVR_multiview", "gl_ViewID_OVR",
+                                      TType(EbtInt, EbpHigh, EvqViewIDOVR, 1));
     }
 
     switch (type)
     {
         case GL_FRAGMENT_SHADER:
         {
-            symbolTable.insert(COMMON_BUILTINS,
-                               new TVariable(NewPoolTString("gl_FragCoord"),
-                                             TType(EbtFloat, EbpMedium, EvqFragCoord, 4)));
-            symbolTable.insert(COMMON_BUILTINS,
-                               new TVariable(NewPoolTString("gl_FrontFacing"),
-                                             TType(EbtBool, EbpUndefined, EvqFrontFacing, 1)));
-            symbolTable.insert(COMMON_BUILTINS,
-                               new TVariable(NewPoolTString("gl_PointCoord"),
-                                             TType(EbtFloat, EbpMedium, EvqPointCoord, 2)));
+            symbolTable.insertVariable(COMMON_BUILTINS, "gl_FragCoord",
+                                       TType(EbtFloat, EbpMedium, EvqFragCoord, 4));
+            symbolTable.insertVariable(COMMON_BUILTINS, "gl_FrontFacing",
+                                       TType(EbtBool, EbpUndefined, EvqFrontFacing, 1));
+            symbolTable.insertVariable(COMMON_BUILTINS, "gl_PointCoord",
+                                       TType(EbtFloat, EbpMedium, EvqPointCoord, 2));
 
-            symbolTable.insert(ESSL1_BUILTINS,
-                               new TVariable(NewPoolTString("gl_FragColor"),
-                                             TType(EbtFloat, EbpMedium, EvqFragColor, 4)));
+            symbolTable.insertVariable(ESSL1_BUILTINS, "gl_FragColor",
+                                       TType(EbtFloat, EbpMedium, EvqFragColor, 4));
             TType fragData(EbtFloat, EbpMedium, EvqFragData, 4);
             if (spec != SH_WEBGL2_SPEC && spec != SH_WEBGL3_SPEC)
             {
@@ -833,35 +822,29 @@
             {
                 fragData.setArraySize(1u);
             }
-            symbolTable.insert(ESSL1_BUILTINS,
-                               new TVariable(NewPoolTString("gl_FragData"), fragData));
+            symbolTable.insertVariable(ESSL1_BUILTINS, "gl_FragData", fragData);
 
             if (resources.EXT_blend_func_extended)
             {
-                symbolTable.insert(
-                    ESSL1_BUILTINS, "GL_EXT_blend_func_extended",
-                    new TVariable(NewPoolTString("gl_SecondaryFragColorEXT"),
-                                  TType(EbtFloat, EbpMedium, EvqSecondaryFragColorEXT, 4)));
+                symbolTable.insertVariableExt(
+                    ESSL1_BUILTINS, "GL_EXT_blend_func_extended", "gl_SecondaryFragColorEXT",
+                    TType(EbtFloat, EbpMedium, EvqSecondaryFragColorEXT, 4));
                 TType secondaryFragData(EbtFloat, EbpMedium, EvqSecondaryFragDataEXT, 4, 1, true);
                 secondaryFragData.setArraySize(resources.MaxDualSourceDrawBuffers);
-                symbolTable.insert(
-                    ESSL1_BUILTINS, "GL_EXT_blend_func_extended",
-                    new TVariable(NewPoolTString("gl_SecondaryFragDataEXT"), secondaryFragData));
+                symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_EXT_blend_func_extended",
+                                              "gl_SecondaryFragDataEXT", secondaryFragData);
             }
 
             if (resources.EXT_frag_depth)
             {
-                symbolTable.insert(
-                    ESSL1_BUILTINS, "GL_EXT_frag_depth",
-                    new TVariable(
-                        NewPoolTString("gl_FragDepthEXT"),
-                        TType(EbtFloat, resources.FragmentPrecisionHigh ? EbpHigh : EbpMedium,
-                              EvqFragDepthEXT, 1)));
+                symbolTable.insertVariableExt(
+                    ESSL1_BUILTINS, "GL_EXT_frag_depth", "gl_FragDepthEXT",
+                    TType(EbtFloat, resources.FragmentPrecisionHigh ? EbpHigh : EbpMedium,
+                          EvqFragDepthEXT, 1));
             }
 
-            symbolTable.insert(ESSL3_BUILTINS,
-                               new TVariable(NewPoolTString("gl_FragDepth"),
-                                             TType(EbtFloat, EbpHigh, EvqFragDepth, 1)));
+            symbolTable.insertVariable(ESSL3_BUILTINS, "gl_FragDepth",
+                                       TType(EbtFloat, EbpHigh, EvqFragDepth, 1));
 
             if (resources.EXT_shader_framebuffer_fetch || resources.NV_shader_framebuffer_fetch)
             {
@@ -870,68 +853,52 @@
 
                 if (resources.EXT_shader_framebuffer_fetch)
                 {
-                    symbolTable.insert(
-                        ESSL1_BUILTINS, "GL_EXT_shader_framebuffer_fetch",
-                        new TVariable(NewPoolTString("gl_LastFragData"), lastFragData));
+                    symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_EXT_shader_framebuffer_fetch",
+                                                  "gl_LastFragData", lastFragData);
                 }
                 else if (resources.NV_shader_framebuffer_fetch)
                 {
-                    symbolTable.insert(
-                        ESSL1_BUILTINS, "GL_NV_shader_framebuffer_fetch",
-                        new TVariable(NewPoolTString("gl_LastFragColor"),
-                                      TType(EbtFloat, EbpMedium, EvqLastFragColor, 4)));
-                    symbolTable.insert(
-                        ESSL1_BUILTINS, "GL_NV_shader_framebuffer_fetch",
-                        new TVariable(NewPoolTString("gl_LastFragData"), lastFragData));
+                    symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_NV_shader_framebuffer_fetch",
+                                                  "gl_LastFragColor",
+                                                  TType(EbtFloat, EbpMedium, EvqLastFragColor, 4));
+                    symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_NV_shader_framebuffer_fetch",
+                                                  "gl_LastFragData", lastFragData);
                 }
             }
             else if (resources.ARM_shader_framebuffer_fetch)
             {
-                symbolTable.insert(ESSL1_BUILTINS, "GL_ARM_shader_framebuffer_fetch",
-                                   new TVariable(NewPoolTString("gl_LastFragColorARM"),
-                                                 TType(EbtFloat, EbpMedium, EvqLastFragColor, 4)));
+                symbolTable.insertVariableExt(ESSL1_BUILTINS, "GL_ARM_shader_framebuffer_fetch",
+                                              "gl_LastFragColorARM",
+                                              TType(EbtFloat, EbpMedium, EvqLastFragColor, 4));
             }
         }
 
         break;
 
         case GL_VERTEX_SHADER:
-            symbolTable.insert(COMMON_BUILTINS,
-                               new TVariable(NewPoolTString("gl_Position"),
-                                             TType(EbtFloat, EbpHigh, EvqPosition, 4)));
-            symbolTable.insert(COMMON_BUILTINS,
-                               new TVariable(NewPoolTString("gl_PointSize"),
-                                             TType(EbtFloat, EbpMedium, EvqPointSize, 1)));
-            symbolTable.insert(ESSL3_BUILTINS,
-                               new TVariable(NewPoolTString("gl_InstanceID"),
-                                             TType(EbtInt, EbpHigh, EvqInstanceID, 1)));
-            symbolTable.insert(ESSL3_BUILTINS,
-                               new TVariable(NewPoolTString("gl_VertexID"),
-                                             TType(EbtInt, EbpHigh, EvqVertexID, 1)));
+            symbolTable.insertVariable(COMMON_BUILTINS, "gl_Position",
+                                       TType(EbtFloat, EbpHigh, EvqPosition, 4));
+            symbolTable.insertVariable(COMMON_BUILTINS, "gl_PointSize",
+                                       TType(EbtFloat, EbpMedium, EvqPointSize, 1));
+            symbolTable.insertVariable(ESSL3_BUILTINS, "gl_InstanceID",
+                                       TType(EbtInt, EbpHigh, EvqInstanceID, 1));
+            symbolTable.insertVariable(ESSL3_BUILTINS, "gl_VertexID",
+                                       TType(EbtInt, EbpHigh, EvqVertexID, 1));
             break;
         case GL_COMPUTE_SHADER:
         {
-            symbolTable.insert(ESSL3_1_BUILTINS,
-                               new TVariable(NewPoolTString("gl_NumWorkGroups"),
-                                             TType(EbtUInt, EbpUndefined, EvqNumWorkGroups, 3)));
-            symbolTable.insert(ESSL3_1_BUILTINS,
-                               new TVariable(NewPoolTString("gl_WorkGroupSize"),
-                                             TType(EbtUInt, EbpUndefined, EvqWorkGroupSize, 3)));
-            symbolTable.insert(ESSL3_1_BUILTINS,
-                               new TVariable(NewPoolTString("gl_WorkGroupID"),
-                                             TType(EbtUInt, EbpUndefined, EvqWorkGroupID, 3)));
-            symbolTable.insert(
-                ESSL3_1_BUILTINS,
-                new TVariable(NewPoolTString("gl_LocalInvocationID"),
-                              TType(EbtUInt, EbpUndefined, EvqLocalInvocationID, 3)));
-            symbolTable.insert(
-                ESSL3_1_BUILTINS,
-                new TVariable(NewPoolTString("gl_GlobalInvocationID"),
-                              TType(EbtUInt, EbpUndefined, EvqGlobalInvocationID, 3)));
-            symbolTable.insert(
-                ESSL3_1_BUILTINS,
-                new TVariable(NewPoolTString("gl_LocalInvocationIndex"),
-                              TType(EbtUInt, EbpUndefined, EvqLocalInvocationIndex, 1)));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_NumWorkGroups",
+                                       TType(EbtUInt, EbpUndefined, EvqNumWorkGroups, 3));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_WorkGroupSize",
+                                       TType(EbtUInt, EbpUndefined, EvqWorkGroupSize, 3));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_WorkGroupID",
+                                       TType(EbtUInt, EbpUndefined, EvqWorkGroupID, 3));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_LocalInvocationID",
+                                       TType(EbtUInt, EbpUndefined, EvqLocalInvocationID, 3));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_GlobalInvocationID",
+                                       TType(EbtUInt, EbpUndefined, EvqGlobalInvocationID, 3));
+            symbolTable.insertVariable(ESSL3_1_BUILTINS, "gl_LocalInvocationIndex",
+                                       TType(EbtUInt, EbpUndefined, EvqLocalInvocationIndex, 1));
         }
         break;
 
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index cb41d00..87abe98 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -32,24 +32,32 @@
 
 const int kWebGLMaxStructNesting = 4;
 
+bool ContainsSampler(const TStructure *structType);
+
 bool ContainsSampler(const TType &type)
 {
     if (IsSampler(type.getBasicType()))
+    {
         return true;
-
+    }
     if (type.getBasicType() == EbtStruct)
     {
-        const TFieldList &fields = type.getStruct()->fields();
-        for (unsigned int i = 0; i < fields.size(); ++i)
-        {
-            if (ContainsSampler(*fields[i]->type()))
-                return true;
-        }
+        return ContainsSampler(type.getStruct());
     }
 
     return false;
 }
 
+bool ContainsSampler(const TStructure *structType)
+{
+    for (const auto &field : structType->fields())
+    {
+        if (ContainsSampler(*field->type()))
+            return true;
+    }
+    return false;
+}
+
 // Get a token from an image argument to use as an error message token.
 const char *GetImageArgumentToken(TIntermTyped *imageNode)
 {
@@ -802,7 +810,7 @@
 {
     if (pType.type == EbtStruct)
     {
-        if (ContainsSampler(*pType.userDef))
+        if (ContainsSampler(pType.userDef))
         {
             std::stringstream reasonStream;
             reasonStream << reason << " (structure contains a sampler)";
@@ -1036,11 +1044,10 @@
     if (needsReservedCheck && !checkIsNotReserved(line, identifier))
         return false;
 
-    (*variable) = new TVariable(&identifier, type);
-    if (!symbolTable.declare(*variable))
+    (*variable) = symbolTable.declareVariable(&identifier, type);
+    if (!(*variable))
     {
         error(line, "redefinition", identifier.c_str());
-        *variable = nullptr;
         return false;
     }
 
@@ -1626,75 +1633,65 @@
                                                  const TString *name,
                                                  const TSymbol *symbol)
 {
-    const TVariable *variable = nullptr;
-
     if (!symbol)
     {
         error(location, "undeclared identifier", name->c_str());
+        return nullptr;
     }
-    else if (!symbol->isVariable())
+
+    if (!symbol->isVariable())
     {
         error(location, "variable expected", name->c_str());
+        return nullptr;
     }
-    else
+
+    const TVariable *variable = static_cast<const TVariable *>(symbol);
+
+    if (symbolTable.findBuiltIn(variable->getName(), mShaderVersion) &&
+        !variable->getExtension().empty())
     {
-        variable = static_cast<const TVariable *>(symbol);
-
-        if (symbolTable.findBuiltIn(variable->getName(), mShaderVersion) &&
-            !variable->getExtension().empty())
-        {
-            checkCanUseExtension(location, variable->getExtension());
-        }
-
-        // Reject shaders using both gl_FragData and gl_FragColor
-        TQualifier qualifier = variable->getType().getQualifier();
-        if (qualifier == EvqFragData || qualifier == EvqSecondaryFragDataEXT)
-        {
-            mUsesFragData = true;
-        }
-        else if (qualifier == EvqFragColor || qualifier == EvqSecondaryFragColorEXT)
-        {
-            mUsesFragColor = true;
-        }
-        if (qualifier == EvqSecondaryFragDataEXT || qualifier == EvqSecondaryFragColorEXT)
-        {
-            mUsesSecondaryOutputs = true;
-        }
-
-        // This validation is not quite correct - it's only an error to write to
-        // both FragData and FragColor. For simplicity, and because users shouldn't
-        // be rewarded for reading from undefined varaibles, return an error
-        // if they are both referenced, rather than assigned.
-        if (mUsesFragData && mUsesFragColor)
-        {
-            const char *errorMessage = "cannot use both gl_FragData and gl_FragColor";
-            if (mUsesSecondaryOutputs)
-            {
-                errorMessage =
-                    "cannot use both output variable sets (gl_FragData, gl_SecondaryFragDataEXT)"
-                    " and (gl_FragColor, gl_SecondaryFragColorEXT)";
-            }
-            error(location, errorMessage, name->c_str());
-        }
-
-        // GLSL ES 3.1 Revision 4, 7.1.3 Compute Shader Special Variables
-        if (getShaderType() == GL_COMPUTE_SHADER && !mComputeShaderLocalSizeDeclared &&
-            qualifier == EvqWorkGroupSize)
-        {
-            error(location,
-                  "It is an error to use gl_WorkGroupSize before declaring the local group size",
-                  "gl_WorkGroupSize");
-        }
+        checkCanUseExtension(location, variable->getExtension());
     }
 
-    if (!variable)
+    // Reject shaders using both gl_FragData and gl_FragColor
+    TQualifier qualifier = variable->getType().getQualifier();
+    if (qualifier == EvqFragData || qualifier == EvqSecondaryFragDataEXT)
     {
-        TType type(EbtFloat, EbpUndefined);
-        TVariable *fakeVariable = new TVariable(name, type);
-        symbolTable.declare(fakeVariable);
-        variable = fakeVariable;
+        mUsesFragData = true;
+    }
+    else if (qualifier == EvqFragColor || qualifier == EvqSecondaryFragColorEXT)
+    {
+        mUsesFragColor = true;
+    }
+    if (qualifier == EvqSecondaryFragDataEXT || qualifier == EvqSecondaryFragColorEXT)
+    {
+        mUsesSecondaryOutputs = true;
     }
 
+    // This validation is not quite correct - it's only an error to write to
+    // both FragData and FragColor. For simplicity, and because users shouldn't
+    // be rewarded for reading from undefined varaibles, return an error
+    // if they are both referenced, rather than assigned.
+    if (mUsesFragData && mUsesFragColor)
+    {
+        const char *errorMessage = "cannot use both gl_FragData and gl_FragColor";
+        if (mUsesSecondaryOutputs)
+        {
+            errorMessage =
+                "cannot use both output variable sets (gl_FragData, gl_SecondaryFragDataEXT)"
+                " and (gl_FragColor, gl_SecondaryFragColorEXT)";
+        }
+        error(location, errorMessage, name->c_str());
+    }
+
+    // GLSL ES 3.1 Revision 4, 7.1.3 Compute Shader Special Variables
+    if (getShaderType() == GL_COMPUTE_SHADER && !mComputeShaderLocalSizeDeclared &&
+        qualifier == EvqWorkGroupSize)
+    {
+        error(location,
+              "It is an error to use gl_WorkGroupSize before declaring the local group size",
+              "gl_WorkGroupSize");
+    }
     return variable;
 }
 
@@ -1704,6 +1701,13 @@
 {
     const TVariable *variable = getNamedVariable(location, name, symbol);
 
+    if (!variable)
+    {
+        TIntermTyped *node = CreateZeroNode(TType(EbtFloat, EbpHigh, EvqConst));
+        node->setLine(location);
+        return node;
+    }
+
     if (variable->getType().getQualifier() == EvqViewIDOVR && IsWebGLBasedSpec(mShaderSpec) &&
         mShaderType == GL_FRAGMENT_SHADER && !isExtensionEnabled("GL_OVR_multiview2"))
     {
@@ -2447,7 +2451,10 @@
     }
 
     const TVariable *variable = getNamedVariable(identifierLoc, identifier, symbol);
-    ASSERT(variable);
+    if (!variable)
+    {
+        return nullptr;
+    }
     const TType &type = variable->getType();
 
     checkInvariantVariableQualifier(typeQualifier.invariant, type.getQualifier(),
@@ -2827,12 +2834,11 @@
         // be used for unused args).
         if (param.name != nullptr)
         {
-            TVariable *variable = new TVariable(param.name, *param.type);
-
             // Insert the parameter in the symbol table.
             if (insertParametersToSymbolTable)
             {
-                if (symbolTable.declare(variable))
+                TVariable *variable = symbolTable.declareVariable(param.name, *param.type);
+                if (variable)
                 {
                     symbol = new TIntermSymbol(variable->getUniqueId(), variable->getName(),
                                                variable->getType());
@@ -3259,8 +3265,7 @@
 
     checkInternalFormatIsNotSpecified(nameLine, blockLayoutQualifier.imageInternalFormat);
 
-    TSymbol *blockNameSymbol = new TInterfaceBlockName(&blockName);
-    if (!symbolTable.declare(blockNameSymbol))
+    if (!symbolTable.declareInterfaceBlockName(&blockName))
     {
         error(nameLine, "redefinition of an interface block name", blockName.c_str());
     }
@@ -3370,10 +3375,13 @@
             // set parent pointer of the field variable
             fieldType->setInterfaceBlock(interfaceBlock);
 
-            TVariable *fieldVariable = new TVariable(&field->name(), *fieldType);
-            fieldVariable->setQualifier(typeQualifier.qualifier);
+            TVariable *fieldVariable = symbolTable.declareVariable(&field->name(), *fieldType);
 
-            if (!symbolTable.declare(fieldVariable))
+            if (fieldVariable)
+            {
+                fieldVariable->setQualifier(typeQualifier.qualifier);
+            }
+            else
             {
                 error(field->line(), "redefinition of an interface block member name",
                       field->name().c_str());
@@ -3385,17 +3393,18 @@
         checkIsNotReserved(instanceLine, *instanceName);
 
         // add a symbol for this interface block
-        TVariable *instanceTypeDef = new TVariable(instanceName, interfaceBlockType, false);
-        instanceTypeDef->setQualifier(typeQualifier.qualifier);
-
-        if (!symbolTable.declare(instanceTypeDef))
+        TVariable *instanceTypeDef = symbolTable.declareVariable(instanceName, interfaceBlockType);
+        if (instanceTypeDef)
+        {
+            instanceTypeDef->setQualifier(typeQualifier.qualifier);
+            symbolId = instanceTypeDef->getUniqueId();
+        }
+        else
         {
             error(instanceLine, "redefinition of an interface block instance name",
                   instanceName->c_str());
         }
-
-        symbolId   = instanceTypeDef->getUniqueId();
-        symbolName = instanceTypeDef->getName();
+        symbolName = *instanceName;
     }
 
     TIntermSymbol *blockSymbol = new TIntermSymbol(symbolId, symbolName, interfaceBlockType);
@@ -4120,7 +4129,7 @@
             type->setArraySize(static_cast<unsigned int>(typeSpecifier.arraySize));
         if (typeSpecifier.getUserDef())
         {
-            type->setStruct(typeSpecifier.getUserDef()->getStruct());
+            type->setStruct(typeSpecifier.getUserDef());
         }
 
         checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *(*fieldList)[i]);
@@ -4135,7 +4144,6 @@
                                                    TFieldList *fieldList)
 {
     TStructure *structure = new TStructure(structName, fieldList);
-    TType *structureType  = new TType(structure);
 
     // Store a bool in the struct if we're at global scope, to allow us to
     // skip the local struct scoping workaround in HLSL.
@@ -4144,8 +4152,7 @@
     if (!structName->empty())
     {
         checkIsNotReserved(nameLine, *structName);
-        TVariable *userTypeDef = new TVariable(structName, *structureType, true);
-        if (!symbolTable.declare(userTypeDef))
+        if (!symbolTable.declareStructType(structure))
         {
             error(nameLine, "redefinition of a struct", structName->c_str());
         }
@@ -4184,7 +4191,7 @@
     }
 
     TTypeSpecifierNonArray typeSpecifierNonArray;
-    typeSpecifierNonArray.initializeStruct(structureType, true, structLine);
+    typeSpecifierNonArray.initializeStruct(structure, true, structLine);
     exitStructDeclaration();
 
     return typeSpecifierNonArray;
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index a7f7e8f..91404fa 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/src/compiler/translator/SymbolTable.cpp
@@ -3,10 +3,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-
-//
-// Symbol table for parsing.  Most functionaliy and main ideas
-// are documented in the header file.
+// Symbol table for parsing. The design principles and most of the functionality are documented in
+// the header file.
 //
 
 #if defined(_MSC_VER)
@@ -271,6 +269,74 @@
     }
 }
 
+TVariable *TSymbolTable::declareVariable(const TString *name, const TType &type)
+{
+    return insertVariable(currentLevel(), name, type);
+}
+
+TVariable *TSymbolTable::declareStructType(TStructure *str)
+{
+    return insertStructType(currentLevel(), str);
+}
+
+TInterfaceBlockName *TSymbolTable::declareInterfaceBlockName(const TString *name)
+{
+    TInterfaceBlockName *blockNameSymbol = new TInterfaceBlockName(name);
+    if (insert(currentLevel(), blockNameSymbol))
+    {
+        return blockNameSymbol;
+    }
+    return nullptr;
+}
+
+TVariable *TSymbolTable::insertVariable(ESymbolLevel level, const char *name, const TType &type)
+{
+    return insertVariable(level, NewPoolTString(name), type);
+}
+
+TVariable *TSymbolTable::insertVariable(ESymbolLevel level, const TString *name, const TType &type)
+{
+    TVariable *var = new TVariable(name, type);
+    if (insert(level, var))
+    {
+        // Do lazy initialization for struct types, so we allocate to the current scope.
+        if (var->getType().getBasicType() == EbtStruct)
+        {
+            var->getType().realize();
+        }
+        return var;
+    }
+    return nullptr;
+}
+
+TVariable *TSymbolTable::insertVariableExt(ESymbolLevel level,
+                                           const char *ext,
+                                           const char *name,
+                                           const TType &type)
+{
+    TVariable *var = new TVariable(NewPoolTString(name), type);
+    if (insert(level, ext, var))
+    {
+        if (var->getType().getBasicType() == EbtStruct)
+        {
+            var->getType().realize();
+        }
+        return var;
+    }
+    return nullptr;
+}
+
+TVariable *TSymbolTable::insertStructType(ESymbolLevel level, TStructure *str)
+{
+    TVariable *var = new TVariable(&str->name(), TType(str), true);
+    if (insert(level, var))
+    {
+        var->getType().realize();
+        return var;
+    }
+    return nullptr;
+}
+
 void TSymbolTable::insertBuiltIn(ESymbolLevel level,
                                  TOperator op,
                                  const char *ext,
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index 48c2dfe..97615d9 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -83,21 +83,12 @@
     TString extension;
 };
 
-// Variable class, meaning a symbol that's not a function.
+// Variable, meaning a symbol that's not a function.
 //
-// There could be a separate class heirarchy for Constant variables;
-// Only one of int, bool, or float, (or none) is correct for
-// any particular use, but it's easy to do this way, and doesn't
-// seem worth having separate classes, and "getConst" can't simply return
-// different values for different types polymorphically, so this is
-// just simple and pragmatic.
+// May store the value of a constant variable of any type (float, int, bool or struct).
 class TVariable : public TSymbol
 {
   public:
-    TVariable(const TString *name, const TType &t, bool uT = false)
-        : TSymbol(name), type(t), userType(uT), unionArray(0)
-    {
-    }
     ~TVariable() override {}
     bool isVariable() const override { return true; }
     TType &getType() { return type; }
@@ -110,8 +101,18 @@
     void shareConstPointer(const TConstantUnion *constArray) { unionArray = constArray; }
 
   private:
+    friend class TSymbolTable;
+
+    TVariable(const TString *name, const TType &t, bool isUserTypeDefinition = false)
+        : TSymbol(name), type(t), userType(isUserTypeDefinition), unionArray(0)
+    {
+    }
+
     TType type;
+
+    // Set to true if this represents a struct type, as opposed to a variable.
     bool userType;
+
     // we are assuming that Pool Allocator will free the memory
     // allocated to unionArray when this object is destroyed.
     const TConstantUnion *unionArray;
@@ -224,9 +225,11 @@
 class TInterfaceBlockName : public TSymbol
 {
   public:
-    TInterfaceBlockName(const TString *name) : TSymbol(name) {}
-
     virtual ~TInterfaceBlockName() {}
+
+  private:
+    friend class TSymbolTable;
+    TInterfaceBlockName(const TString *name) : TSymbol(name) {}
 };
 
 class TSymbolTableLevel
@@ -319,15 +322,22 @@
         precisionStack.pop_back();
     }
 
-    bool declare(TSymbol *symbol) { return insert(currentLevel(), symbol); }
+    // The declare* entry points are used when parsing and declare symbols at the current scope.
+    // They return the created symbol in case the declaration was successful, and nullptr if the
+    // declaration failed due to redefinition.
+    TVariable *declareVariable(const TString *name, const TType &type);
+    TVariable *declareStructType(TStructure *str);
+    TInterfaceBlockName *declareInterfaceBlockName(const TString *name);
 
-    bool insert(ESymbolLevel level, TSymbol *symbol) { return table[level]->insert(symbol); }
-
-    bool insert(ESymbolLevel level, const char *ext, TSymbol *symbol)
-    {
-        symbol->relateToExtension(ext);
-        return table[level]->insert(symbol);
-    }
+    // The insert* entry points are used when initializing the symbol table with built-ins.
+    // They return the created symbol in case the declaration was successful, and nullptr if the
+    // declaration failed due to redefinition.
+    TVariable *insertVariable(ESymbolLevel level, const char *name, const TType &type);
+    TVariable *insertVariableExt(ESymbolLevel level,
+                                 const char *ext,
+                                 const char *name,
+                                 const TType &type);
+    TVariable *insertStructType(ESymbolLevel level, TStructure *str);
 
     bool insertConstInt(ESymbolLevel level, const char *name, int value, TPrecision precision)
     {
@@ -444,8 +454,6 @@
         return table[currentLevel() - 1];
     }
 
-    void dump(TInfoSink &infoSink) const;
-
     void setDefaultPrecision(TBasicType type, TPrecision prec)
     {
         int indexOfLastElement = static_cast<int>(precisionStack.size()) - 1;
@@ -488,6 +496,16 @@
   private:
     ESymbolLevel currentLevel() const { return static_cast<ESymbolLevel>(table.size() - 1); }
 
+    TVariable *insertVariable(ESymbolLevel level, const TString *name, const TType &type);
+
+    bool insert(ESymbolLevel level, TSymbol *symbol) { return table[level]->insert(symbol); }
+
+    bool insert(ESymbolLevel level, const char *ext, TSymbol *symbol)
+    {
+        symbol->relateToExtension(ext);
+        return table[level]->insert(symbol);
+    }
+
     // Used to insert unmangled functions to check redeclaration of built-ins in ESSL 3.00 and
     // above.
     void insertUnmangledBuiltInName(const char *name, ESymbolLevel level);
diff --git a/src/compiler/translator/Types.cpp b/src/compiler/translator/Types.cpp
index e968962..3d230de 100644
--- a/src/compiler/translator/Types.cpp
+++ b/src/compiler/translator/Types.cpp
@@ -130,7 +130,7 @@
     ASSERT(primarySize <= 4);
     ASSERT(secondarySize <= 4);
     if (p.getUserDef())
-        structure = p.getUserDef()->getStruct();
+        structure = p.getUserDef();
 }
 
 bool TStructure::equals(const TStructure &other) const
diff --git a/src/compiler/translator/Types.h b/src/compiler/translator/Types.h
index 0bad198..e95964c 100644
--- a/src/compiler/translator/Types.h
+++ b/src/compiler/translator/Types.h
@@ -517,7 +517,7 @@
     TBasicType type;
     unsigned char primarySize;    // size of vector or cols of matrix
     unsigned char secondarySize;  // rows of matrix
-    TType *userDef;
+    TStructure *userDef;
     TSourceLoc line;
 
     // true if the type was defined by a struct specifier rather than a reference to a type name.
@@ -534,7 +534,7 @@
         isStructSpecifier = false;
     }
 
-    void initializeStruct(TType *aUserDef, bool aIsStructSpecifier, const TSourceLoc &aLine)
+    void initializeStruct(TStructure *aUserDef, bool aIsStructSpecifier, const TSourceLoc &aLine)
     {
         type              = EbtStruct;
         primarySize       = 1;
@@ -610,7 +610,7 @@
     unsigned char getPrimarySize() const { return typeSpecifierNonArray.primarySize; }
     unsigned char getSecondarySize() const { return typeSpecifierNonArray.secondarySize; }
 
-    const TType *getUserDef() const { return typeSpecifierNonArray.userDef; }
+    TStructure *getUserDef() const { return typeSpecifierNonArray.userDef; }
     const TSourceLoc &getLine() const { return typeSpecifierNonArray.line; }
 
     bool isStructSpecifier() const { return typeSpecifierNonArray.isStructSpecifier; }
@@ -622,7 +622,7 @@
             return false;
         }
 
-        return typeSpecifierNonArray.userDef->isStructureContainingArrays();
+        return typeSpecifierNonArray.userDef->containsArrays();
     }
 
     bool isStructureContainingType(TBasicType t) const
@@ -632,7 +632,7 @@
             return false;
         }
 
-        return typeSpecifierNonArray.userDef->isStructureContainingType(t);
+        return typeSpecifierNonArray.userDef->containsType(t);
     }
 
     bool isUnsizedArray() const { return array && arraySize == 0; }
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index f11b8c4..f6c182b 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -1175,7 +1175,7 @@
     | TYPE_NAME {
         // This is for user defined type names. The lexical phase looked up the type.
         TType& structure = static_cast<TVariable*>($1.symbol)->getType();
-        $$.initializeStruct(&structure, false, @1);
+        $$.initializeStruct(structure.getStruct(), false, @1);
     }
     ;
 
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index d84416f..ed83499 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -4395,7 +4395,7 @@
     {
         // This is for user defined type names. The lexical phase looked up the type.
         TType& structure = static_cast<TVariable*>((yyvsp[0].lex).symbol)->getType();
-        (yyval.interm.typeSpecifierNonArray).initializeStruct(&structure, false, (yylsp[0]));
+        (yyval.interm.typeSpecifierNonArray).initializeStruct(structure.getStruct(), false, (yylsp[0]));
     }
 
     break;