Clean up inserting variables to symbol table

src/compiler/translator/ParseContext.cpp

@@ -32,21 +32,29 @@ namespace
 
 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)
+        return ContainsSampler(type.getStruct());
+    }
+
+    return false;
+}
+
+bool ContainsSampler(const TStructure *structType)
+{
+    for (const auto &field : structType->fields())
     {
-            if (ContainsSampler(*fields[i]->type()))
+        if (ContainsSampler(*field->type()))
             return true;
     }
-    }
-
     return false;
 }
 
@@ -802,7 +810,7 @@ bool TParseContext::checkIsNotOpaqueType(const TSourceLoc &line,
 {
     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 @@ bool TParseContext::declareVariable(const TSourceLoc &line,
     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,19 +1633,19 @@ const TVariable *TParseContext::getNamedVariable(const TSourceLoc &location,
                                                  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
-    {
-        variable = static_cast<const TVariable *>(symbol);
+
+    const TVariable *variable = static_cast<const TVariable *>(symbol);
 
     if (symbolTable.findBuiltIn(variable->getName(), mShaderVersion) &&
         !variable->getExtension().empty())
@@ -1685,16 +1692,6 @@ const TVariable *TParseContext::getNamedVariable(const TSourceLoc &location,
               "It is an error to use gl_WorkGroupSize before declaring the local group size",
               "gl_WorkGroupSize");
     }
-    }
-
-    if (!variable)
-    {
-        TType type(EbtFloat, EbpUndefined);
-        TVariable *fakeVariable = new TVariable(name, type);
-        symbolTable.declare(fakeVariable);
-        variable = fakeVariable;
-    }
-
     return variable;
 }
 
@@ -1704,6 +1701,13 @@ TIntermTyped *TParseContext::parseVariableIdentifier(const TSourceLoc &location,
 {
     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 @@ TIntermInvariantDeclaration *TParseContext::parseInvariantDeclaration(
     }
 
     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 @@ TIntermFunctionPrototype *TParseContext::createPrototypeNodeFromFunction(
         // 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 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
 
     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 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
             // 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 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
         checkIsNotReserved(instanceLine, *instanceName);
 
         // add a symbol for this interface block
-        TVariable *instanceTypeDef = new TVariable(instanceName, interfaceBlockType, false);
+        TVariable *instanceTypeDef = symbolTable.declareVariable(instanceName, interfaceBlockType);
+        if (instanceTypeDef)
+        {
             instanceTypeDef->setQualifier(typeQualifier.qualifier);
-
-        if (!symbolTable.declare(instanceTypeDef))
+            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 @@ TFieldList *TParseContext::addStructDeclaratorList(const TPublicType &typeSpecif
             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 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
                                                    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 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
     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 TParseContext::addStructure(const TSourceLoc &structLine,
     }
 
     TTypeSpecifierNonArray typeSpecifierNonArray;
-    typeSpecifierNonArray.initializeStruct(structureType, true, structLine);
+    typeSpecifierNonArray.initializeStruct(structure, true, structLine);
     exitStructDeclaration();
 
     return typeSpecifierNonArray;

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 @@ const TType *VectorType(const TType *type, int size)
     }
 }
 
+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,

src/compiler/translator/SymbolTable.h

@@ -83,21 +83,12 @@ class TSymbol : angle::NonCopyable
     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 @@ class TVariable : public TSymbol
     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 TFunction : public TSymbol
 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 @@ class TSymbolTable : angle::NonCopyable
         precisionStack.pop_back();
     }
 
-    bool declare(TSymbol *symbol) { return insert(currentLevel(), symbol); }
-
-    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 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);
+
+    // 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 @@ class TSymbolTable : angle::NonCopyable
         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 @@ class TSymbolTable : angle::NonCopyable
   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);

src/compiler/translator/Types.cpp

@@ -130,7 +130,7 @@ TType::TType(const TPublicType &p)
     ASSERT(primarySize <= 4);
     ASSERT(secondarySize <= 4);
     if (p.getUserDef())
-        structure = p.getUserDef()->getStruct();
+        structure = p.getUserDef();
 }
 
 bool TStructure::equals(const TStructure &other) const

src/compiler/translator/Types.h

@@ -517,7 +517,7 @@ struct TTypeSpecifierNonArray
     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 @@ struct TTypeSpecifierNonArray
         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 @@ struct TPublicType
     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 @@ struct TPublicType
             return false;
         }
 
-        return typeSpecifierNonArray.userDef->isStructureContainingArrays();
+        return typeSpecifierNonArray.userDef->containsArrays();
     }
 
     bool isStructureContainingType(TBasicType t) const
@@ -632,7 +632,7 @@ struct TPublicType
             return false;
         }
 
-        return typeSpecifierNonArray.userDef->isStructureContainingType(t);
+        return typeSpecifierNonArray.userDef->containsType(t);
     }
 
     bool isUnsizedArray() const { return array && arraySize == 0; }

src/compiler/translator/glslang.y

@@ -1175,7 +1175,7 @@ type_specifier_nonarray
     | 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);
     }
     ;
 

src/compiler/translator/glslang_tab.cpp

@@ -4395,7 +4395,7 @@ yyreduce:
     {
         // 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;