Get all the scoping rules right for ES and non ES, name hiding, built-in overriding, etc.

Test/300scope.vert

+#version 300 es
+
+int f(int a, int b, int c)
+{
+	int a = b;  // ERROR, redefinition
+
+    {
+		float a = float(a) + 1.0;
+    }
+
+	return a;
+}
+
+int f(int a, int b, int c);  // okay to redeclare
+
+bool b;
+float b(int a);      // ERROR: redefinition
+
+float f;             // ERROR: redefinition
+float tan;           // ERROR: redefines built-in function
+float sin(float x);  // ERROR: can't overload built-in functions
+float cos(float x)   // ERROR: can't overload built-in functions
+{
+	return 1.0;
+}
+
+invariant gl_Position;
+
+void main()
+{
+    int g();    // ERROR: no local function declarations
+	g();
+
+    float sin;  // okay
+	sin;
+
+    f(1,2,3);
+
+    float f;    // hides f()
+    f = 3.0;
+
+    gl_Position = vec4(f);
+
+    for (int f = 0; f < 10; ++f)
+        ++f;
+
+    int x = 1;
+    { 
+        float x = 2.0, /* 2nd x visible here */ y = x; // y is initialized to 2
+        int z = z; // ERROR: z not previously defined.
+    }
+    {
+        int x = x; // x is initialized to '1'
+    }
+
+    struct S 
+    { 
+        int x; 
+    };
+    {
+        S S = S(0); // 'S' is only visible as a struct and constructor 
+        S.x;        // 'S' is now visible as a variable
+    }
+}

Test/430scope.vert

+#version 430 core
+
+int f(int a, int b, int c)
+{
+	int a = b;  // ERROR, redefinition
+
+    {
+		float a = float(a) + 1.0; // okay
+    }
+
+	return a;
+}
+
+int f(int a, int b, int c);  // okay to redeclare
+
+bool b;
+float b(int a);      // ERROR: redefinition
+
+float f;             // ERROR: redefinition
+float tan;           // okay, hides built-in function
+float sin(float x);  // okay, can overload built-in functions
+float cos(float x)   // okay, can overload built-in functions
+{
+	return 1.0;
+}
+
+invariant gl_Position;
+
+void main()
+{
+    int g();    // okay
+    g();
+
+    float sin; // okay
+    sin;
+
+    f(1,2,3);
+
+    float f;    // hides f()
+    f = 3.0;
+
+    gl_Position = vec4(f);
+
+    for (int f = 0; f < 10; ++f)
+        ++f;
+
+    int x = 1;
+    { 
+        float x = 2.0, /* 2nd x visible here */ y = x; // y is initialized to 2
+        int z = z; // ERROR: z not previously defined.
+    }
+    {
+        int x = x; // x is initialized to '1'
+    }
+
+    struct S 
+    { 
+        int x; 
+    };
+    {
+        S S = S(0); // 'S' is only visible as a struct and constructor 
+        S.x;        // 'S' is now visible as a variable
+    }
+}

Test/testlist

@@ -37,3 +37,5 @@ forwardRef.frag
 uint.frag
 switch.frag
 tokenLength.vert
+300scope.vert
+430scope.vert

glslang/MachineIndependent/ParseHelper.cpp

@@ -223,6 +223,47 @@ void C_DECL TParseContext::error(TSourceLoc nLine, const char *szReason, const c
     ++numErrors;
 }
 
+TIntermTyped* TParseContext::handleVariable(int line, TSymbol* symbol, TString* string)
+{
+    TIntermTyped* node = 0;
+
+    TAnonMember* anon = symbol ? symbol->getAsAnonMember() : 0;
+    if (anon) {
+        // it was a member of an anonymous container, have to insert its dereference
+        TVariable* variable = anon->getAnonContainer().getAsVariable();
+        TIntermTyped* container = intermediate.addSymbol(variable->getUniqueId(), variable->getName(), variable->getType(), line);
+        constUnion* unionArray = new constUnion[1];
+        unionArray->setUConst(anon->getMemberNumber());
+        TIntermTyped* constNode = intermediate.addConstantUnion(unionArray, TType(EbtUint, EvqConst), line);
+
+        node = intermediate.addIndex(EOpIndexDirectStruct, container, constNode, line);
+        node->setType(*(*variable->getType().getStruct())[anon->getMemberNumber()].type);
+    } else {
+        // The symbol table search was done in the lexical phase, but
+        // if this is a new symbol, it wouldn't have found it.
+        const TVariable* variable = symbol ? symbol->getAsVariable() : 0;
+        if (symbol && ! variable) {
+            error(line, "variable name expected", string->c_str(), "");
+            recover();
+        }
+
+        if (! variable)
+            variable = new TVariable(string, TType(EbtVoid));
+
+        // don't delete $1.string, it's used by error recovery, and the pool
+        // pop will reclaim the memory
+
+        if (variable->getType().getQualifier().storage == EvqConst ) {
+            constUnion* constArray = variable->getConstUnionPointer();
+            TType t(variable->getType());
+            node = intermediate.addConstantUnion(constArray, t, line);
+        } else
+            node = intermediate.addSymbol(variable->getUniqueId(), variable->getName(), variable->getType(), line);
+    }
+
+    return node;
+}
+
 //
 // Same error message for all places assignments don't work.
 //
@@ -935,9 +976,8 @@ bool TParseContext::arrayErrorCheck(int line, TString& identifier, const TPublic
     // because reserved arrays can be redeclared.
     //
 
-    bool builtIn = false; 
     bool sameScope = false;
-    TSymbol* symbol = symbolTable.find(identifier, &builtIn, &sameScope);
+    TSymbol* symbol = symbolTable.find(identifier, 0, &sameScope);
     if (symbol == 0 || !sameScope) {
         if (reservedErrorCheck(line, identifier))
             return true;
@@ -992,8 +1032,7 @@ bool TParseContext::arrayErrorCheck(int line, TString& identifier, const TPublic
 
 bool TParseContext::arraySetMaxSize(TIntermSymbol *node, TType* type, int size, bool updateFlag, TSourceLoc line)
 {
-    bool builtIn = false;
-    TSymbol* symbol = symbolTable.find(node->getSymbol(), &builtIn);
+    TSymbol* symbol = symbolTable.find(node->getSymbol());
     if (symbol == 0) {
         error(line, " undeclared identifier", node->getSymbol().c_str(), "");
         return true;
@@ -1011,7 +1050,7 @@ bool TParseContext::arraySetMaxSize(TIntermSymbol *node, TType* type, int size,
     // special casing to test index value of gl_TexCoord. If the accessed index is >= gl_MaxTextureCoords
     // its an error
     if (node->getSymbol() == "gl_TexCoord") {
-        TSymbol* texCoord = symbolTable.find("gl_MaxTextureCoords", &builtIn);
+        TSymbol* texCoord = symbolTable.find("gl_MaxTextureCoords");
         if (! texCoord || ! texCoord->getAsVariable()) {
             infoSink.info.message(EPrefixInternalError, "gl_MaxTextureCoords not defined", line);
             return true;

glslang/MachineIndependent/ParseHelper.h

@@ -103,6 +103,7 @@ struct TParseContext {
     bool reservedErrorCheck(int line, const TString& identifier);
     void recover();
 
+    TIntermTyped* handleVariable(int line, TSymbol* symbol, TString* string);
     bool parseVectorFields(const TString&, int vecSize, TVectorFields&, int line);
     void assignError(int line, const char* op, TString left, TString right);
     void unaryOpError(int line, const char* op, TString operand);

glslang/MachineIndependent/ShaderLang.cpp

@@ -169,11 +169,6 @@ bool AddContextSpecificSymbols(const TBuiltInResource* resources, TInfoSink& inf
     return true;
 }
 
-//
-// Driver must call this first, once, before doing any other
-// compiler/linker operations.
-//
-
 void SetupBuiltinSymbolTable(int version, EProfile profile)
 {
     TInfoSink infoSink;
@@ -188,6 +183,11 @@ void SetupBuiltinSymbolTable(int version, EProfile profile)
     SetGlobalPoolAllocatorPtr(*builtInPoolAllocator);
 
     TSymbolTable symTables[EShLangCount];
+    if (profile == EEsProfile) {
+        for (int stage = 0; stage < EShLangCount; ++stage)
+            symTables[stage].setNoBuiltInRedeclarations();
+    }
+
     GenerateBuiltInSymbolTable(infoSink, symTables, version, profile);
 
     SetGlobalPoolAllocatorPtr(*PerProcessGPA);

glslang/MachineIndependent/SymbolTable.cpp

@@ -268,9 +268,8 @@ TSymbolTableLevel* TSymbolTableLevel::clone(TStructureMap& remapper)
 	TSymbolTableLevel *symTableLevel = new TSymbolTableLevel();
     symTableLevel->anonId = anonId;
 	tLevel::iterator iter;
-	for (iter = level.begin(); iter != level.end(); ++iter) {
+	for (iter = level.begin(); iter != level.end(); ++iter)
 		symTableLevel->insert(*iter->second->clone(remapper));
-	}
 
 	return symTableLevel;
 }
@@ -279,7 +278,7 @@ void TSymbolTable::copyTable(const TSymbolTable& copyOf)
 {
 	TStructureMap remapper;
 	uniqueId = copyOf.uniqueId;
-	for (unsigned int i = 0; i < copyOf.table.size(); ++i) {
+    noBuiltInRedeclarations = copyOf.noBuiltInRedeclarations;
+	for (unsigned int i = 0; i < copyOf.table.size(); ++i)
 		table.push_back(copyOf.table[i]->clone(remapper));
-	}
 }

glslang/MachineIndependent/SymbolTable.h

@@ -252,10 +252,11 @@ public:
     bool insert(TSymbol& symbol)
     {
         //
-        // returning true means symbol was added to the table
+        // returning true means symbol was added to the table with no semantic errors
         //
         tInsertResult result;
-        if (symbol.getName() == "") {
+        const TString& name = symbol.getName();
+        if (name == "") {
             // An empty name means an anonymous container, exposing its members to the external scope.
             // Give it a name and insert its members in the symbol table, pointing to the container.
             char buf[20];
@@ -273,9 +274,25 @@ public:
 
             return isOkay;
         } else {
-            result = level.insert(tLevelPair(symbol.getMangledName(), &symbol));
-
-            return result.second;
+            // Check for redefinition errors:
+            // - STL itself will tell us if there is a direct name collision at this level
+            // - additionally, check for function/variable name collisions
+            // - for ES, for overriding or hiding built-in function
+            const TString& insertName = symbol.getMangledName();
+            if (symbol.getAsFunction()) {
+                // make sure there isn't a variable of this name
+                if (level.find(name) != level.end())
+                    return false;
+
+                // insert, and whatever happens is okay
+                level.insert(tLevelPair(insertName, &symbol));
+
+                return true;
+            } else {
+                result = level.insert(tLevelPair(insertName, &symbol));
+
+                return result.second;
+            }
         }
     }
 
@@ -288,6 +305,20 @@ public:
             return (*it).second;
     }
 
+    bool hasFunctionName(const TString& name) const
+    {
+        tLevel::const_iterator candidate = level.lower_bound(name);
+        if (candidate != level.end()) {
+            const TString& candidateName = (*candidate).first;
+            TString::size_type parenAt = candidateName.find_first_of('(');
+            if (parenAt != candidateName.npos && candidateName.substr(0, parenAt) == name)
+
+                return true;
+        }
+
+        return false;
+    }
+
     // Use this to do a lazy 'push' of precision defaults the first time
     // a precision statement is seen in a new scope.  Leave it at 0 for
     // when no push was needed.  Thus, it is not the current defaults,
@@ -334,7 +365,7 @@ protected:
 
 class TSymbolTable {
 public:
-    TSymbolTable() : uniqueId(0)
+    TSymbolTable() : uniqueId(0), noBuiltInRedeclarations(false)
     {
         //
         // The symbol table cannot be used until push() is called, but
@@ -346,23 +377,26 @@ public:
     {
         table.push_back(symTable.table[0]);
         uniqueId = symTable.uniqueId;
+        noBuiltInRedeclarations = symTable.noBuiltInRedeclarations;
     }
     ~TSymbolTable()
     {
-        // level 0 is always built In symbols, so we never pop that out
+        // level 0 is always built-in symbols, so we never pop that out
         while (table.size() > 1)
             pop(0);
     }
 
     //
     // When the symbol table is initialized with the built-ins, there should
-    // 'push' calls, so that built-ins are at level 0 and the shader
-    // globals are at level 1.
+    // 'push' calls, so that built-in shared across all compiles are at level 0 
+    // built-ins specific to a compile are at level 1 and the shader
+    // globals are at level 2.
     //
     bool isEmpty() { return table.size() == 0; }
     bool atBuiltInLevel() { return atSharedBuiltInLevel() || atDynamicBuiltInLevel(); }
     bool atSharedBuiltInLevel() { return table.size() == 1; }	
     bool atGlobalLevel() { return table.size() <= 3; }
+    void setNoBuiltInRedeclarations() { noBuiltInRedeclarations = true; }
     
     void push()
     {
@@ -379,6 +413,19 @@ public:
     bool insert(TSymbol& symbol)
     {
         symbol.setUniqueId(++uniqueId);
+
+        if (symbol.getAsVariable()) {
+            // make sure there isn't a function of this name
+            if (table[currentLevel()]->hasFunctionName(symbol.getName()))
+                return false;
+            if (atGlobalLevel() && currentLevel() > 0 && noBuiltInRedeclarations) {
+                if (table[0]->hasFunctionName(symbol.getName()))
+                    return false;
+                if (currentLevel() > 1 && table[1]->hasFunctionName(symbol.getName()))
+                    return false;
+            }
+        }
+
         return table[currentLevel()]->insert(symbol);
     }
 
@@ -392,7 +439,7 @@ public:
         } while (symbol == 0 && level >= 0);
         level++;
         if (builtIn)
-            *builtIn = level == 0;
+            *builtIn = level < 2;
         if (sameScope)
             *sameScope = level == currentLevel();
         return symbol;
@@ -414,6 +461,7 @@ protected:
 
     std::vector<TSymbolTableLevel*> table;
     int uniqueId;     // for unique identification in code generation
+    bool noBuiltInRedeclarations;
 };
 
 #endif // _SYMBOL_TABLE_INCLUDED_

glslang/MachineIndependent/glslang.y

@@ -218,45 +218,7 @@ extern void yyerror(const char*);
 
 variable_identifier
     : IDENTIFIER {
-        // The symbol table search was done in the lexical phase, but
-        // if this is a new symbol, it won't find it, which is okay at this
-        // point in the grammar.
-        TSymbol* symbol = $1.symbol;
-        TAnonMember* anon = symbol ? symbol->getAsAnonMember() : 0;
-        if (anon) {
-            // it was a member of an anonymous container, have to insert its dereference
-            TVariable* variable = anon->getAnonContainer().getAsVariable();
-            TIntermTyped* container = parseContext.intermediate.addSymbol(variable->getUniqueId(),
-                                                                          variable->getName(),
-                                                                          variable->getType(), $1.line);
-            constUnion* unionArray = new constUnion[1];
-            unionArray->setUConst(anon->getMemberNumber());
-            TIntermTyped* constNode = parseContext.intermediate.addConstantUnion(unionArray, TType(EbtUint, EvqConst), $1.line);
-
-            $$ = parseContext.intermediate.addIndex(EOpIndexDirectStruct, container, constNode, $1.line);
-            $$->setType(*(*variable->getType().getStruct())[anon->getMemberNumber()].type);
-        } else {
-            const TVariable* variable = symbol ? symbol->getAsVariable() : 0;
-            if (symbol && ! variable) {
-                parseContext.error($1.line, "variable name expected", $1.string->c_str(), "");
-                parseContext.recover();
-            }
-
-            if (! variable)
-                variable = new TVariable($1.string, TType(EbtVoid));
-
-            // don't delete $1.string, it's used by error recovery, and the pool
-            // pop will reclaim the memory
-
-            if (variable->getType().getQualifier().storage == EvqConst ) {
-                constUnion* constArray = variable->getConstUnionPointer();
-                TType t(variable->getType());
-                $$ = parseContext.intermediate.addConstantUnion(constArray, t, $1.line);
-            } else
-                $$ = parseContext.intermediate.addSymbol(variable->getUniqueId(),
-                                                         variable->getName(),
-                                                         variable->getType(), $1.line);
-        }
+        $$ = parseContext.handleVariable($1.line, $1.symbol, $1.string);
     }
     ;
 
@@ -1240,15 +1202,24 @@ identifier_list
 
 function_prototype
     : function_declarator RIGHT_PAREN  {
+        // ES can't declare prototypes inside functions
+        if (! parseContext.symbolTable.atGlobalLevel())
+            parseContext.requireProfile($2.line, static_cast<EProfileMask>(~EEsProfileMask), "local function declaration");
+
         //
         // Multiple declarations of the same function are allowed.
         //
         // If this is a definition, the definition production code will check for redefinitions
         // (we don't know at this point if it's a definition or not).
         //
-        // Redeclarations are allowed.  But, return types and parameter qualifiers must match.
+        // Redeclarations (full prototype match) are allowed.  But, return types and parameter qualifiers must match.
         //
-        TSymbol* symbol = parseContext.symbolTable.find($1->getMangledName());
+        // ES does not allow redeclaring or hiding of built-in functions.
+        //
+        bool builtIn;
+        TSymbol* symbol = parseContext.symbolTable.find($1->getMangledName(), &builtIn);
+        if (symbol && symbol->getAsFunction() && builtIn)
+            parseContext.requireProfile($2.line, static_cast<EProfileMask>(~EEsProfileMask), "redeclaration of built-in function");
         TFunction* prevDec = symbol ? symbol->getAsFunction() : 0;
         if (prevDec) {
             if (prevDec->getReturnType() != $1->getReturnType()) {
@@ -1272,7 +1243,10 @@ function_prototype
         $$.function = $1;
         $$.line = $2.line;
 
-        parseContext.symbolTable.insert(*$$.function);
+        if (! parseContext.symbolTable.insert(*$$.function)) {
+            parseContext.error($2.line, "illegal redeclaration", $$.function->getName().c_str(), "");
+            parseContext.recover();
+        }
     }
     ;