Clean up storing interface blocks in the symbol table

src/compiler/translator/CollectVariables.cpp

@@ -573,7 +573,7 @@ void CollectVariablesTraverser::setCommonVariableProperties(const TType &type,
 
         const TFieldList &fields = structure->fields();
 
-        for (TField *field : fields)
+        for (const TField *field : fields)
         {
             // Regardless of the variable type (uniform, in/out etc.) its fields are always plain
             // ShaderVariable objects.

src/compiler/translator/FlagStd140Structs.cpp

@@ -9,6 +9,7 @@
 #include "compiler/translator/FlagStd140Structs.h"
 
 #include "compiler/translator/IntermTraverse.h"
+#include "compiler/translator/SymbolTable.h"
 
 namespace sh
 {

src/compiler/translator/Initialize.cpp

@@ -1015,29 +1015,31 @@ void IdentifyBuiltIns(sh::GLenum type,
 
             // Add built-in interface block gl_PerVertex and the built-in array gl_in.
             // TODO(jiawei.shao@intel.com): implement GL_OES_geometry_point_size.
-            const TString *glPerVertexString = NewPoolTString("gl_PerVertex");
-            symbolTable.insertInterfaceBlockNameExt(ESSL3_1_BUILTINS, extension, glPerVertexString);
-
-            TFieldList *fieldList    = new TFieldList();
+            TFieldList *glPerVertexFieldList = new TFieldList();
             TSourceLoc zeroSourceLoc = {0, 0, 0, 0};
             TField *glPositionField  = new TField(new TType(EbtFloat, EbpHigh, EvqPosition, 4),
                                                  NewPoolTString("gl_Position"), zeroSourceLoc);
-            fieldList->push_back(glPositionField);
+            glPerVertexFieldList->push_back(glPositionField);
 
-            TInterfaceBlock *glInBlock =
-                new TInterfaceBlock(glPerVertexString, fieldList, TLayoutQualifier::Create());
+            const TString *glPerVertexString    = NewPoolTString("gl_PerVertex");
+            TInterfaceBlock *glPerVertexInBlock = new TInterfaceBlock(
+                &symbolTable, glPerVertexString, glPerVertexFieldList, TLayoutQualifier::Create());
+            glPerVertexInBlock->relateToExtension(extension);
+            symbolTable.insertInterfaceBlock(ESSL3_1_BUILTINS, glPerVertexInBlock);
 
             // The array size of gl_in is undefined until we get a valid input primitive
             // declaration.
-            TType glInType(glInBlock, EvqPerVertexIn, TLayoutQualifier::Create());
+            TType glInType(glPerVertexInBlock, EvqPerVertexIn, TLayoutQualifier::Create());
             glInType.makeArray(0u);
             symbolTable.insertVariableExt(ESSL3_1_BUILTINS, extension, "gl_in", glInType);
 
+            TInterfaceBlock *glPerVertexOutBlock = new TInterfaceBlock(
+                &symbolTable, glPerVertexString, glPerVertexFieldList, TLayoutQualifier::Create());
             TType glPositionType(EbtFloat, EbpHigh, EvqPosition, 4);
-            glPositionType.setInterfaceBlock(
-                new TInterfaceBlock(glPerVertexString, fieldList, TLayoutQualifier::Create()));
+            glPositionType.setInterfaceBlock(glPerVertexOutBlock);
             symbolTable.insertVariableExt(ESSL3_1_BUILTINS, extension, "gl_Position",
                                           glPositionType);
+
             symbolTable.insertVariableExt(ESSL3_1_BUILTINS, extension, "gl_PrimitiveIDIn",
                                           TType(EbtInt, EbpHigh, EvqPrimitiveIDIn, 1));
             symbolTable.insertVariableExt(ESSL3_1_BUILTINS, extension, "gl_InvocationID",

src/compiler/translator/ParseContext.cpp

@@ -3713,11 +3713,6 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
 
     checkInternalFormatIsNotSpecified(nameLine, blockLayoutQualifier.imageInternalFormat);
 
-    if (!symbolTable.declareInterfaceBlockName(&blockName))
-    {
-        error(nameLine, "redefinition of an interface block name", blockName.c_str());
-    }
-
     // check for sampler types and apply layout qualifiers
     for (size_t memberIndex = 0; memberIndex < fieldList->size(); ++memberIndex)
     {
@@ -3814,7 +3809,12 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
     }
 
     TInterfaceBlock *interfaceBlock =
-        new TInterfaceBlock(&blockName, fieldList, blockLayoutQualifier);
+        new TInterfaceBlock(&symbolTable, &blockName, fieldList, blockLayoutQualifier);
+    if (!symbolTable.declareInterfaceBlock(interfaceBlock))
+    {
+        error(nameLine, "redefinition of an interface block name", blockName.c_str());
+    }
+
     TType interfaceBlockType(interfaceBlock, typeQualifier.qualifier, blockLayoutQualifier);
     if (arrayIndex != nullptr)
     {

src/compiler/translator/SymbolTable.cpp

@@ -70,6 +70,17 @@ void TStructure::setName(const TString &name)
     *mutableName         = name;
 }
 
+TInterfaceBlock::TInterfaceBlock(TSymbolTable *symbolTable,
+                                 const TString *name,
+                                 const TFieldList *fields,
+                                 const TLayoutQualifier &layoutQualifier)
+    : TSymbol(symbolTable, name),
+      TFieldListCollection(fields),
+      mBlockStorage(layoutQualifier.blockStorage),
+      mBinding(layoutQualifier.binding)
+{
+}
+
 //
 // Functions have buried pointers to delete.
 //
@@ -315,27 +326,9 @@ bool TSymbolTable::declareStructType(TStructure *str)
     return insertStructType(currentLevel(), str);
 }
 
-TInterfaceBlockName *TSymbolTable::declareInterfaceBlockName(const TString *name)
-{
-    TInterfaceBlockName *blockNameSymbol = new TInterfaceBlockName(this, name);
-    if (insert(currentLevel(), blockNameSymbol))
-    {
-        return blockNameSymbol;
-    }
-    return nullptr;
-}
-
-TInterfaceBlockName *TSymbolTable::insertInterfaceBlockNameExt(ESymbolLevel level,
-                                                               TExtension ext,
-                                                               const TString *name)
+bool TSymbolTable::declareInterfaceBlock(TInterfaceBlock *interfaceBlock)
 {
-    TInterfaceBlockName *blockNameSymbol = new TInterfaceBlockName(this, name);
-    blockNameSymbol->relateToExtension(ext);
-    if (insert(level, blockNameSymbol))
-    {
-        return blockNameSymbol;
-    }
-    return nullptr;
+    return insert(currentLevel(), interfaceBlock);
 }
 
 TVariable *TSymbolTable::insertVariable(ESymbolLevel level, const char *name, const TType &type)
@@ -379,11 +372,13 @@ TVariable *TSymbolTable::insertVariableExt(ESymbolLevel level,
 bool TSymbolTable::insertStructType(ESymbolLevel level, TStructure *str)
 {
     ASSERT(str);
-    if (insert(level, str))
-    {
-        return true;
-    }
-    return false;
+    return insert(level, str);
+}
+
+bool TSymbolTable::insertInterfaceBlock(ESymbolLevel level, TInterfaceBlock *interfaceBlock)
+{
+    ASSERT(interfaceBlock);
+    return insert(level, interfaceBlock);
 }
 
 void TSymbolTable::insertBuiltIn(ESymbolLevel level,

src/compiler/translator/SymbolTable.h

@@ -122,6 +122,26 @@ class TStructure : public TSymbol, public TFieldListCollection
     bool mAtGlobalScope;
 };
 
+// Interface block. Note that this contains the block name, not the instance name. Interface block
+// instances are stored as TVariable.
+class TInterfaceBlock : public TSymbol, public TFieldListCollection
+{
+  public:
+    TInterfaceBlock(TSymbolTable *symbolTable,
+                    const TString *name,
+                    const TFieldList *fields,
+                    const TLayoutQualifier &layoutQualifier);
+
+    TLayoutBlockStorage blockStorage() const { return mBlockStorage; }
+    int blockBinding() const { return mBinding; }
+
+  private:
+    TLayoutBlockStorage mBlockStorage;
+    int mBinding;
+
+    // Note that we only record matrix packing on a per-field granularity.
+};
+
 // Immutable version of TParameter.
 struct TConstParameter
 {
@@ -226,20 +246,6 @@ class TFunction : public TSymbol
     bool mHasPrototypeDeclaration;
 };
 
-// Reserved interface block name. Note that this simply reserves the block name, not the instance
-// name. Interface block instances are stored as TVariable.
-class TInterfaceBlockName : public TSymbol
-{
-  public:
-    virtual ~TInterfaceBlockName() {}
-
-  private:
-    friend class TSymbolTable;
-    TInterfaceBlockName(TSymbolTable *symbolTable, const TString *name) : TSymbol(symbolTable, name)
-    {
-    }
-};
-
 class TSymbolTableLevel
 {
   public:
@@ -338,7 +344,7 @@ class TSymbolTable : angle::NonCopyable
     // false if the declaration failed due to redefinition.
     TVariable *declareVariable(const TString *name, const TType &type);
     bool declareStructType(TStructure *str);
-    TInterfaceBlockName *declareInterfaceBlockName(const TString *name);
+    bool declareInterfaceBlock(TInterfaceBlock *interfaceBlock);
 
     // The insert* entry points are used when initializing the symbol table with built-ins.
     // They return the created symbol / true in case the declaration was successful, and nullptr /
@@ -349,9 +355,7 @@ class TSymbolTable : angle::NonCopyable
                                  const char *name,
                                  const TType &type);
     bool insertStructType(ESymbolLevel level, TStructure *str);
-    TInterfaceBlockName *insertInterfaceBlockNameExt(ESymbolLevel level,
-                                                     TExtension ext,
-                                                     const TString *name);
+    bool insertInterfaceBlock(ESymbolLevel level, TInterfaceBlock *interfaceBlock);
 
     bool insertConstInt(ESymbolLevel level, const char *name, int value, TPrecision precision)
     {

src/compiler/translator/Types.cpp

@@ -932,16 +932,6 @@ int TFieldListCollection::calculateDeepestNesting() const
     return 1 + maxNesting;
 }
 
-TInterfaceBlock::TInterfaceBlock(const TString *name,
-                                 const TFieldList *fields,
-                                 const TLayoutQualifier &layoutQualifier)
-    : TFieldListCollection(fields),
-      mName(name),
-      mBlockStorage(layoutQualifier.blockStorage),
-      mBinding(layoutQualifier.binding)
-{
-}
-
 // TPublicType implementation.
 void TPublicType::initialize(const TTypeSpecifierNonArray &typeSpecifier, TQualifier q)
 {

src/compiler/translator/Types.h

@@ -19,6 +19,7 @@ namespace sh
 
 struct TPublicType;
 class TType;
+class TInterfaceBlock;
 class TStructure;
 class TSymbol;
 class TIntermSymbol;
@@ -80,27 +81,6 @@ class TFieldListCollection : angle::NonCopyable
     mutable TString mMangledFieldList;
 };
 
-class TInterfaceBlock : public TFieldListCollection
-{
-  public:
-    POOL_ALLOCATOR_NEW_DELETE();
-    TInterfaceBlock(const TString *name,
-                    const TFieldList *fields,
-                    const TLayoutQualifier &layoutQualifier);
-
-    const TString &name() const { return *mName; }
-    TLayoutBlockStorage blockStorage() const { return mBlockStorage; }
-    int blockBinding() const { return mBinding; }
-
-  private:
-    const TString *mName;  // Name of the block, not the instance name. Instance name is only stored
-                           // in the interface block symbols.
-    TLayoutBlockStorage mBlockStorage;
-    int mBinding;
-
-    // Note that we only record matrix packing on a per-field granularity.
-};
-
 //
 // Base class for things that have a type.
 //