Clean up TSymbol initialization
Now TSymbol objects always get their id when they are constructed. The
id cannot be changed after the TSymbol is created.
This makes it simpler to insert both mangled and unmangled versions of
a function to the symbol table. These can now both share the same
TSymbol object, unlike before, when inserting the same symbol twice
would have changed its symbol id.
This requires changes to function definition parsing: function
definition nodes now share any symbol created by previous prototype
declarations of the function. The parameters on the symbol get set to
the parameters in the function definition header.
BUG=angleproject:1490
TEST=angle_unittests
Change-Id: I8e600e9b5e5de27d64b85c5042cfd23ff02abe63
Reviewed-on: https://chromium-review.googlesource.com/396838
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 36a2d84..6b41588 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2006,30 +2006,32 @@
}
}
-TIntermAggregate *TParseContext::addFunctionPrototypeDeclaration(const TFunction &function,
+TIntermAggregate *TParseContext::addFunctionPrototypeDeclaration(const TFunction &parsedFunction,
const TSourceLoc &location)
{
- // Note: symbolTableFunction could be the same as function if this is the first declaration.
- // Either way the instance in the symbol table is used to track whether the function is declared
- // multiple times.
- TFunction *symbolTableFunction =
- static_cast<TFunction *>(symbolTable.find(function.getMangledName(), getShaderVersion()));
- if (symbolTableFunction->hasPrototypeDeclaration() && mShaderVersion == 100)
+ // Note: function found from the symbol table could be the same as parsedFunction if this is the
+ // first declaration. Either way the instance in the symbol table is used to track whether the
+ // function is declared multiple times.
+ TFunction *function = static_cast<TFunction *>(
+ symbolTable.find(parsedFunction.getMangledName(), getShaderVersion()));
+ if (function->hasPrototypeDeclaration() && mShaderVersion == 100)
{
// ESSL 1.00.17 section 4.2.7.
// Doesn't apply to ESSL 3.00.4: see section 4.2.3.
error(location, "duplicate function prototype declarations are not allowed", "function");
}
- symbolTableFunction->setHasPrototypeDeclaration();
+ function->setHasPrototypeDeclaration();
TIntermAggregate *prototype = new TIntermAggregate;
- prototype->setType(function.getReturnType());
- prototype->setName(function.getMangledName());
- prototype->setFunctionId(function.getUniqueId());
+ // TODO(oetuaho@nvidia.com): Instead of converting the function information here, the node could
+ // point to the data that already exists in the symbol table.
+ prototype->setType(function->getReturnType());
+ prototype->setName(function->getMangledName());
+ prototype->setFunctionId(function->getUniqueId());
- for (size_t i = 0; i < function.getParamCount(); i++)
+ for (size_t i = 0; i < function->getParamCount(); i++)
{
- const TConstParameter ¶m = function.getParam(i);
+ const TConstParameter ¶m = function->getParam(i);
if (param.name != 0)
{
TVariable variable(param.name, *param.type);
@@ -2090,48 +2092,56 @@
return functionNode;
}
-void TParseContext::parseFunctionPrototype(const TSourceLoc &location,
- TFunction *function,
- TIntermAggregate **aggregateOut)
+void TParseContext::parseFunctionDefinitionHeader(const TSourceLoc &location,
+ TFunction **function,
+ TIntermAggregate **aggregateOut)
{
+ ASSERT(function);
+ ASSERT(*function);
const TSymbol *builtIn =
- symbolTable.findBuiltIn(function->getMangledName(), getShaderVersion());
+ symbolTable.findBuiltIn((*function)->getMangledName(), getShaderVersion());
if (builtIn)
{
- error(location, "built-in functions cannot be redefined", function->getName().c_str());
+ error(location, "built-in functions cannot be redefined", (*function)->getName().c_str());
}
-
- TFunction *prevDec =
- static_cast<TFunction *>(symbolTable.find(function->getMangledName(), getShaderVersion()));
- //
- // Note: 'prevDec' could be 'function' if this is the first time we've seen function
- // as it would have just been put in the symbol table. Otherwise, we're looking up
- // an earlier occurance.
- //
- if (prevDec->isDefined())
+ else
{
- // Then this function already has a body.
- error(location, "function already has a body", function->getName().c_str());
+ TFunction *prevDec = static_cast<TFunction *>(
+ symbolTable.find((*function)->getMangledName(), getShaderVersion()));
+
+ // Note: 'prevDec' could be 'function' if this is the first time we've seen function as it
+ // would have just been put in the symbol table. Otherwise, we're looking up an earlier
+ // occurance.
+ if (*function != prevDec)
+ {
+ // Swap the parameters of the previous declaration to the parameters of the function
+ // definition (parameter names may differ).
+ prevDec->swapParameters(**function);
+
+ // The function definition will share the same symbol as any previous declaration.
+ *function = prevDec;
+ }
+
+ if ((*function)->isDefined())
+ {
+ error(location, "function already has a body", (*function)->getName().c_str());
+ }
+
+ (*function)->setDefined();
}
- prevDec->setDefined();
- //
- // Overload the unique ID of the definition to be the same unique ID as the declaration.
- // Eventually we will probably want to have only a single definition and just swap the
- // arguments to be the definition's arguments.
- //
- function->setUniqueId(prevDec->getUniqueId());
// Raise error message if main function takes any parameters or return anything other than void
- if (function->getName() == "main")
+ if ((*function)->getName() == "main")
{
- if (function->getParamCount() > 0)
+ if ((*function)->getParamCount() > 0)
{
- error(location, "function cannot take any parameter(s)", function->getName().c_str());
+ error(location, "function cannot take any parameter(s)",
+ (*function)->getName().c_str());
}
- if (function->getReturnType().getBasicType() != EbtVoid)
+ if ((*function)->getReturnType().getBasicType() != EbtVoid)
{
- error(location, "", function->getReturnType().getBasicString(),
+ error(location, "", (*function)->getReturnType().getBasicString(),
"main function cannot return a value");
}
}
@@ -2139,7 +2149,7 @@
//
// Remember the return type for later checking for RETURN statements.
//
- mCurrentFunctionType = &(prevDec->getReturnType());
+ mCurrentFunctionType = &((*function)->getReturnType());
mFunctionReturnsValue = false;
//
@@ -2151,9 +2161,9 @@
// knows where to find parameters.
//
TIntermAggregate *paramNodes = new TIntermAggregate;
- for (size_t i = 0; i < function->getParamCount(); i++)
+ for (size_t i = 0; i < (*function)->getParamCount(); i++)
{
- const TConstParameter ¶m = function->getParam(i);
+ const TConstParameter ¶m = (*function)->getParam(i);
if (param.name != 0)
{
TVariable *variable = new TVariable(param.name, *param.type);
@@ -2211,7 +2221,7 @@
{
if (prevDec->getReturnType() != function->getReturnType())
{
- error(location, "overloaded functions must have the same return type",
+ error(location, "function must have the same return type in all of its declarations",
function->getReturnType().getBasicString());
}
for (size_t i = 0; i < prevDec->getParamCount(); ++i)
@@ -2219,7 +2229,8 @@
if (prevDec->getParam(i).type->getQualifier() !=
function->getParam(i).type->getQualifier())
{
- error(location, "overloaded functions must have the same parameter qualifiers",
+ error(location,
+ "function must have the same parameter qualifiers in all of its declarations",
function->getParam(i).type->getQualifierString());
}
}
@@ -2239,9 +2250,7 @@
else
{
// Insert the unmangled name to detect potential future redefinition as a variable.
- TFunction *newFunction =
- new TFunction(NewPoolTString(function->getName().c_str()), &function->getReturnType());
- symbolTable.getOuterLevel()->insertUnmangled(newFunction);
+ symbolTable.getOuterLevel()->insertUnmangled(function);
}
// We're at the inner scope level of the function's arguments and body statement.
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index aeb901c..c7dda08 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -264,15 +264,15 @@
TIntermTyped *initializer);
void parseGlobalLayoutQualifier(const TTypeQualifierBuilder &typeQualifierBuilder);
- TIntermAggregate *addFunctionPrototypeDeclaration(const TFunction &function,
+ TIntermAggregate *addFunctionPrototypeDeclaration(const TFunction &parsedFunction,
const TSourceLoc &location);
TIntermAggregate *addFunctionDefinition(const TFunction &function,
TIntermAggregate *functionPrototype,
TIntermBlock *functionBody,
const TSourceLoc &location);
- void parseFunctionPrototype(const TSourceLoc &location,
- TFunction *function,
- TIntermAggregate **aggregateOut);
+ void parseFunctionDefinitionHeader(const TSourceLoc &location,
+ TFunction **function,
+ TIntermAggregate **aggregateOut);
TFunction *parseFunctionDeclarator(const TSourceLoc &location,
TFunction *function);
TFunction *parseFunctionHeader(const TPublicType &type,
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index 059c5c7..aa23776 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/src/compiler/translator/SymbolTable.cpp
@@ -21,13 +21,33 @@
int TSymbolTable::uniqueIdCounter = 0;
+TSymbol::TSymbol(const TString *n) : uniqueId(TSymbolTable::nextUniqueId()), name(n)
+{
+}
+
//
// Functions have buried pointers to delete.
//
TFunction::~TFunction()
{
+ clearParameters();
+}
+
+void TFunction::clearParameters()
+{
for (TParamList::iterator i = parameters.begin(); i != parameters.end(); ++i)
delete (*i).type;
+ parameters.clear();
+ mangledName = nullptr;
+}
+
+void TFunction::swapParameters(const TFunction ¶metersSource)
+{
+ clearParameters();
+ for (auto parameter : parametersSource.parameters)
+ {
+ addParameter(parameter);
+ }
}
const TString *TFunction::buildMangledName() const
@@ -53,8 +73,6 @@
bool TSymbolTableLevel::insert(TSymbol *symbol)
{
- symbol->setUniqueId(TSymbolTable::nextUniqueId());
-
// returning true means symbol was added to the table
tInsertResult result = level.insert(tLevelPair(symbol->getMangledName(), symbol));
@@ -63,8 +81,6 @@
bool TSymbolTableLevel::insertUnmangled(TFunction *function)
{
- function->setUniqueId(TSymbolTable::nextUniqueId());
-
// returning true means symbol was added to the table
tInsertResult result = level.insert(tLevelPair(function->getName(), function));
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index f092412..51a82f7 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -43,11 +43,8 @@
{
public:
POOL_ALLOCATOR_NEW_DELETE();
- TSymbol(const TString *n)
- : uniqueId(0),
- name(n)
- {
- }
+ TSymbol(const TString *n);
+
virtual ~TSymbol()
{
// don't delete name, it's from the pool
@@ -69,10 +66,6 @@
{
return false;
}
- void setUniqueId(int id)
- {
- uniqueId = id;
- }
int getUniqueId() const
{
return uniqueId;
@@ -87,7 +80,7 @@
}
private:
- int uniqueId; // For real comparing during code generation
+ const int uniqueId;
const TString *name;
TString extension;
};
@@ -229,6 +222,8 @@
mangledName = nullptr;
}
+ void swapParameters(const TFunction ¶metersSource);
+
const TString &getMangledName() const override
{
if (mangledName == nullptr)
@@ -262,6 +257,8 @@
}
private:
+ void clearParameters();
+
const TString *buildMangledName() const;
typedef TVector<TConstParameter> TParamList;
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index b11c7d5..7476f62 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -1477,7 +1477,7 @@
function_definition
: function_prototype {
- context->parseFunctionPrototype(@1, $1.function, &$1.intermAggregate);
+ context->parseFunctionDefinitionHeader(@1, &($1.function), &$1.intermAggregate);
}
compound_statement_no_new_scope {
$$ = context->addFunctionDefinition(*($1.function), $1.intermAggregate, $3, @1);
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index c0512e2..d9b8ada 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -4658,7 +4658,7 @@
case 268:
{
- context->parseFunctionPrototype((yylsp[0]), (yyvsp[0].interm).function, &(yyvsp[0].interm).intermAggregate);
+ context->parseFunctionDefinitionHeader((yylsp[0]), &((yyvsp[0].interm).function), &(yyvsp[0].interm).intermAggregate);
}
break;