Add unittests to verify invariant doesn't leak
This is a followup CL of
https://chromium-review.googlesource.com/366720. Unittests is added to
check invariant status does not leak across shaders.
This CL also moves mInvariantVaryings and mGlobalInvariant from
TSymbolTable to TSymbolTableLevel. So at the end of a compilation, the
levels pop, and the settings will be cleared and will not affect the
next compilation.
Change-Id: I1199fade7a637276ab149ab9a599621b9977298b
Reviewed-on: https://chromium-review.googlesource.com/370844
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 06dc6cd..0257dd3 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -208,7 +208,6 @@
const int compileOptions)
{
clearResults();
- symbolTable.clearInvariantVaryings();
ASSERT(numStrings > 0);
ASSERT(GetGlobalPoolAllocator());
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 1561382..eac0702 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -410,12 +410,14 @@
// Both test, and if necessary spit out an error, to see if we are currently
// globally scoped.
-void TParseContext::checkIsAtGlobalLevel(const TSourceLoc &line, const char *token)
+bool TParseContext::checkIsAtGlobalLevel(const TSourceLoc &line, const char *token)
{
if (!symbolTable.atGlobalLevel())
{
error(line, "only allowed at global scope", token);
+ return false;
}
+ return true;
}
// For now, keep it simple: if it starts "gl_", it's reserved, independent
@@ -1672,7 +1674,8 @@
const TSymbol *symbol)
{
// invariant declaration
- checkIsAtGlobalLevel(invariantLoc, "invariant varying");
+ if (!checkIsAtGlobalLevel(invariantLoc, "invariant varying"))
+ return nullptr;
if (!symbol)
{
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 0fb6e37..05ce2e7 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -138,7 +138,7 @@
bool checkCanBeLValue(const TSourceLoc &line, const char *op, TIntermTyped *node);
void checkIsConst(TIntermTyped *node);
void checkIsScalarInteger(TIntermTyped *node, const char *token);
- void checkIsAtGlobalLevel(const TSourceLoc &line, const char *token);
+ bool checkIsAtGlobalLevel(const TSourceLoc &line, const char *token);
bool checkConstructorArguments(const TSourceLoc &line,
TIntermNode *argumentsNode,
const TFunction &function,
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index d01e20c..60e0b99 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -296,6 +296,7 @@
typedef std::pair<tLevel::iterator, bool> tInsertResult;
TSymbolTableLevel()
+ : mGlobalInvariant(false)
{
}
~TSymbolTableLevel();
@@ -307,8 +308,22 @@
TSymbol *find(const TString &name) const;
+ void addInvariantVarying(const std::string &name)
+ {
+ mInvariantVaryings.insert(name);
+ }
+
+ bool isVaryingInvariant(const std::string &name)
+ {
+ return (mGlobalInvariant || mInvariantVaryings.count(name) > 0);
+ }
+
+ void setGlobalInvariant(bool invariant) { mGlobalInvariant = invariant; }
+
protected:
tLevel level;
+ std::set<std::string> mInvariantVaryings;
+ bool mGlobalInvariant;
};
// Define ESymbolLevel as int rather than an enum since level can go
@@ -326,7 +341,6 @@
{
public:
TSymbolTable()
- : mGlobalInvariant(false)
{
// The symbol table cannot be used until push() is called, but
// the lack of an initial call to push() can be used to detect
@@ -348,7 +362,7 @@
}
bool atGlobalLevel() const
{
- return currentLevel() <= GLOBAL_LEVEL;
+ return currentLevel() == GLOBAL_LEVEL;
}
void push()
{
@@ -477,25 +491,24 @@
// "invariant varying_name;".
void addInvariantVarying(const std::string &originalName)
{
- mInvariantVaryings.insert(originalName);
+ ASSERT(atGlobalLevel());
+ table[currentLevel()]->addInvariantVarying(originalName);
}
-
- void clearInvariantVaryings()
- {
- mInvariantVaryings.clear();
- }
-
// If this returns false, the varying could still be invariant
// if it is set as invariant during the varying variable
// declaration - this piece of information is stored in the
// variable's type, not here.
bool isVaryingInvariant(const std::string &originalName) const
{
- return (mGlobalInvariant ||
- mInvariantVaryings.count(originalName) > 0);
+ ASSERT(atGlobalLevel());
+ return table[currentLevel()]->isVaryingInvariant(originalName);
}
- void setGlobalInvariant(bool invariant) { mGlobalInvariant = invariant; }
+ void setGlobalInvariant(bool invariant)
+ {
+ ASSERT(atGlobalLevel());
+ table[currentLevel()]->setGlobalInvariant(invariant);
+ }
static int nextUniqueId()
{
@@ -525,9 +538,6 @@
std::set<std::string> mUnmangledBuiltinNames;
- std::set<std::string> mInvariantVaryings;
- bool mGlobalInvariant;
-
static int uniqueIdCounter;
};