From fbe01520b6248cf6d386816ba1f81e66f7cb5cbf Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Wed, 12 Jun 2013 06:13:17 +0000 Subject: [PATCH] Get all the scoping rules right for ES and non ES, name hiding, built-in overriding, etc. git-svn-id: https://cvs.khronos.org/svn/repos/ogl/trunk/ecosystem/public/sdk/tools/glslang@21948 e7fa87d3-cd2b-0410-9028-fcbf551c1848 --- Test/300scope.vert | 64 +++++++++++++++++++++ Test/430scope.vert | 64 +++++++++++++++++++++ Test/testlist | 2 + glslang/MachineIndependent/ParseHelper.cpp | 49 ++++++++++++++-- glslang/MachineIndependent/ParseHelper.h | 1 + glslang/MachineIndependent/ShaderLang.cpp | 10 ++-- glslang/MachineIndependent/SymbolTable.cpp | 7 +-- glslang/MachineIndependent/SymbolTable.h | 66 +++++++++++++++++++--- glslang/MachineIndependent/glslang.y | 58 ++++++------------- 9 files changed, 256 insertions(+), 65 deletions(-) create mode 100644 Test/300scope.vert create mode 100644 Test/430scope.vert diff --git a/Test/300scope.vert b/Test/300scope.vert new file mode 100644 index 00000000..263b62ce --- /dev/null +++ b/Test/300scope.vert @@ -0,0 +1,64 @@ +#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 + } +} diff --git a/Test/430scope.vert b/Test/430scope.vert new file mode 100644 index 00000000..00685adf --- /dev/null +++ b/Test/430scope.vert @@ -0,0 +1,64 @@ +#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 + } +} diff --git a/Test/testlist b/Test/testlist index 654f87fa..9c8932cc 100644 --- a/Test/testlist +++ b/Test/testlist @@ -37,3 +37,5 @@ forwardRef.frag uint.frag switch.frag tokenLength.vert +300scope.vert +430scope.vert diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 36585e8f..d62a6b08 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/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; diff --git a/glslang/MachineIndependent/ParseHelper.h b/glslang/MachineIndependent/ParseHelper.h index 10b31357..07ebc960 100644 --- a/glslang/MachineIndependent/ParseHelper.h +++ b/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); diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index 8df3352f..e3d17350 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/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); diff --git a/glslang/MachineIndependent/SymbolTable.cpp b/glslang/MachineIndependent/SymbolTable.cpp index 3b222f65..27d6fc71 100644 --- a/glslang/MachineIndependent/SymbolTable.cpp +++ b/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)); - } } diff --git a/glslang/MachineIndependent/SymbolTable.h b/glslang/MachineIndependent/SymbolTable.h index f6a500fa..10ffaa08 100644 --- a/glslang/MachineIndependent/SymbolTable.h +++ b/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)); + // 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; - return result.second; + // 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 table; int uniqueId; // for unique identification in code generation + bool noBuiltInRedeclarations; }; #endif // _SYMBOL_TABLE_INCLUDED_ diff --git a/glslang/MachineIndependent/glslang.y b/glslang/MachineIndependent/glslang.y index a8c16ee9..3a10d122 100644 --- a/glslang/MachineIndependent/glslang.y +++ b/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(~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(~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(); + } } ;