PP: Non-functional: Make a proper class out of the atom <-> string mapping.

glslang/Include/revision.h

@@ -2,5 +2,5 @@
 // For the version, it uses the latest git tag followed by the number of commits.
 // For the date, it uses the current date (when then script is run).
 
-#define GLSLANG_REVISION "Overload400-PrecQual.1720"
+#define GLSLANG_REVISION "Overload400-PrecQual.1721"
 #define GLSLANG_DATE "21-Dec-2016"

glslang/MachineIndependent/preprocessor/Pp.cpp

@@ -105,7 +105,7 @@ int TPpContext::CPPdefine(TPpToken* ppToken)
     }
 
     // save the macro name
-    const int defAtom = LookUpAddString(ppToken->name);
+    const int defAtom = atomStrings.getAddAtom(ppToken->name);
 
     // gather parameters to the macro, between (...)
     token = scanToken(ppToken);
@@ -121,7 +121,7 @@ int TPpContext::CPPdefine(TPpToken* ppToken)
                 return token;
             }
             mac.emptyArgs = 0;
-            const int argAtom = LookUpAddString(ppToken->name);
+            const int argAtom = atomStrings.getAddAtom(ppToken->name);
 
             // check for duplication of parameter name
             bool duplicate = false;
@@ -162,10 +162,10 @@ int TPpContext::CPPdefine(TPpToken* ppToken)
             // "Two replacement lists are identical if and only if the preprocessing tokens in both have the same number,
             // ordering, spelling, and white-space separation, where all white-space separations are considered identical."
             if (existing->args.size() != mac.args.size() || existing->emptyArgs != mac.emptyArgs)
-                parseContext.ppError(defineLoc, "Macro redefined; different number of arguments:", "#define", GetAtomString(defAtom));
+                parseContext.ppError(defineLoc, "Macro redefined; different number of arguments:", "#define", atomStrings.getString(defAtom));
             else {
                 if (existing->args != mac.args)
-                    parseContext.ppError(defineLoc, "Macro redefined; different argument names:", "#define", GetAtomString(defAtom));
+                    parseContext.ppError(defineLoc, "Macro redefined; different argument names:", "#define", atomStrings.getString(defAtom));
                 RewindTokenStream(existing->body);
                 RewindTokenStream(mac.body);
                 int newToken;
@@ -176,7 +176,7 @@ int TPpContext::CPPdefine(TPpToken* ppToken)
                     oldToken = ReadToken(existing->body, &oldPpToken);
                     newToken = ReadToken(mac.body, &newPpToken);
                     if (oldToken != newToken || oldPpToken != newPpToken) {
-                        parseContext.ppError(defineLoc, "Macro redefined; different substitutions:", "#define", GetAtomString(defAtom));
+                        parseContext.ppError(defineLoc, "Macro redefined; different substitutions:", "#define", atomStrings.getString(defAtom));
                         break; 
                     }
                 } while (newToken > 0);
@@ -201,7 +201,7 @@ int TPpContext::CPPundef(TPpToken* ppToken)
 
     parseContext.reservedPpErrorCheck(ppToken->loc, ppToken->name, "#undef");
 
-    MacroSymbol* macro = lookupMacroDef(LookUpString(ppToken->name));
+    MacroSymbol* macro = lookupMacroDef(atomStrings.getAtom(ppToken->name));
     if (macro != nullptr)
         macro->undef = 1;
     token = scanToken(ppToken);
@@ -236,7 +236,7 @@ int TPpContext::CPPelse(int matchelse, TPpToken* ppToken)
         if ((token = scanToken(ppToken)) != PpAtomIdentifier)
             continue;
 
-        int nextAtom = LookUpString(ppToken->name);
+        int nextAtom = atomStrings.getAtom(ppToken->name);
         if (nextAtom == PpAtomIf || nextAtom == PpAtomIfdef || nextAtom == PpAtomIfndef) {
             depth++; 
             ifdepth++; 
@@ -405,7 +405,7 @@ int TPpContext::eval(int token, int precedence, bool shortCircuit, int& res, boo
                 return token;
             }
 
-            MacroSymbol* macro = lookupMacroDef(LookUpString(ppToken->name));
+            MacroSymbol* macro = lookupMacroDef(atomStrings.getAtom(ppToken->name));
             res = macro != nullptr ? !macro->undef : 0;
             token = scanToken(ppToken);
             if (needclose) {
@@ -557,7 +557,7 @@ int TPpContext::CPPifdef(int defined, TPpToken* ppToken)
         else 
             parseContext.ppError(ppToken->loc, "must be followed by macro name", "#ifndef", "");
     } else {
-        MacroSymbol* macro = lookupMacroDef(LookUpString(ppToken->name));
+        MacroSymbol* macro = lookupMacroDef(atomStrings.getAtom(ppToken->name));
         token = scanToken(ppToken);
         if (token != '\n') {
             parseContext.ppError(ppToken->loc, "unexpected tokens following #ifdef directive - expected a newline", "#ifdef", "");
@@ -651,7 +651,7 @@ int TPpContext::CPPline(TPpToken* ppToken)
                 // We need to save a copy of the string instead of pointing
                 // to the name field of the token since the name field
                 // will likely be overwritten by the next token scan.
-                sourceName = GetAtomString(LookUpAddString(ppToken->name));
+                sourceName = atomStrings.getString(atomStrings.getAddAtom(ppToken->name));
                 parseContext.setCurrentSourceName(sourceName);
                 hasFile = true;
                 token = scanToken(ppToken);
@@ -690,7 +690,7 @@ int TPpContext::CPPerror(TPpToken* ppToken)
         } else if (token == PpAtomIdentifier || token == PpAtomConstString) {
             message.append(ppToken->name);
         } else {
-            message.append(GetAtomString(token));
+            message.append(atomStrings.getString(token));
         }
         message.append(" ");
         token = scanToken(ppToken);
@@ -767,7 +767,7 @@ int TPpContext::CPPversion(TPpToken* ppToken)
         parseContext.notifyVersion(line, versionNumber, nullptr);
         return token;
     } else {
-        int profileAtom = LookUpString(ppToken->name);
+        int profileAtom = atomStrings.getAtom(ppToken->name);
         if (profileAtom != PpAtomCore &&
             profileAtom != PpAtomCompatibility &&
             profileAtom != PpAtomEs)
@@ -831,7 +831,7 @@ int TPpContext::readCPPline(TPpToken* ppToken)
     int token = scanToken(ppToken);
 
     if (token == PpAtomIdentifier) {
-        switch (LookUpString(ppToken->name)) {
+        switch (atomStrings.getAtom(ppToken->name)) {
         case PpAtomDefine:
             token = CPPdefine(ppToken);
             break;
@@ -922,7 +922,7 @@ TPpContext::TokenStream* TPpContext::PrescanMacroArg(TokenStream& arg, TPpToken*
     RewindTokenStream(arg);
     do {
         token = ReadToken(arg, ppToken);
-        if (token == PpAtomIdentifier && lookupMacroDef(LookUpString(ppToken->name)) != nullptr)
+        if (token == PpAtomIdentifier && lookupMacroDef(atomStrings.getAtom(ppToken->name)) != nullptr)
             break;
     } while (token != EndOfInput);
 
@@ -992,7 +992,7 @@ int TPpContext::tMacroInput::scan(TPpToken* ppToken)
     if (token == PpAtomIdentifier) {
         int i;
         for (i = mac->args.size() - 1; i >= 0; i--)
-            if (strcmp(pp->GetAtomString(mac->args[i]), ppToken->name) == 0)
+            if (strcmp(pp->atomStrings.getString(mac->args[i]), ppToken->name) == 0)
                 break;
         if (i >= 0) {
             TokenStream* arg = expandedArgs[i];
@@ -1060,7 +1060,7 @@ int TPpContext::tZeroInput::scan(TPpToken* ppToken)
 int TPpContext::MacroExpand(TPpToken* ppToken, bool expandUndef, bool newLineOkay)
 {
     ppToken->space = false;
-    int macroAtom = LookUpString(ppToken->name);
+    int macroAtom = atomStrings.getAtom(ppToken->name);
     switch (macroAtom) {
     case PpAtomLineMacro:
         ppToken->ival = parseContext.getCurrentLoc().line;
@@ -1116,7 +1116,7 @@ int TPpContext::MacroExpand(TPpToken* ppToken, bool expandUndef, bool newLineOka
                 token = scanToken(ppToken);
         }
         if (token != '(') {
-            parseContext.ppError(loc, "expected '(' following", "macro expansion", GetAtomString(macroAtom));
+            parseContext.ppError(loc, "expected '(' following", "macro expansion", atomStrings.getString(macroAtom));
             UngetToken(token, ppToken);
             delete in;
             return 0;
@@ -1134,20 +1134,20 @@ int TPpContext::MacroExpand(TPpToken* ppToken, bool expandUndef, bool newLineOka
             while (1) {
                 token = scanToken(ppToken);
                 if (token == EndOfInput) {
-                    parseContext.ppError(loc, "End of input in macro", "macro expansion", GetAtomString(macroAtom));
+                    parseContext.ppError(loc, "End of input in macro", "macro expansion", atomStrings.getString(macroAtom));
                     delete in;
                     return 0;
                 }
                 if (token == '\n') {
                     if (! newLineOkay) {
-                        parseContext.ppError(loc, "End of line in macro substitution:", "macro expansion", GetAtomString(macroAtom));
+                        parseContext.ppError(loc, "End of line in macro substitution:", "macro expansion", atomStrings.getString(macroAtom));
                         delete in;
                         return 0;
                     }
                     continue;
                 }
                 if (token == '#') {
-                    parseContext.ppError(ppToken->loc, "unexpected '#'", "macro expansion", GetAtomString(macroAtom));
+                    parseContext.ppError(ppToken->loc, "unexpected '#'", "macro expansion", atomStrings.getString(macroAtom));
                     delete in;
                     return 0;
                 }
@@ -1172,7 +1172,7 @@ int TPpContext::MacroExpand(TPpToken* ppToken, bool expandUndef, bool newLineOka
         } while (arg < in->mac->args.size());
 
         if (arg < in->mac->args.size())
-            parseContext.ppError(loc, "Too few args in Macro", "macro expansion", GetAtomString(macroAtom));
+            parseContext.ppError(loc, "Too few args in Macro", "macro expansion", atomStrings.getString(macroAtom));
         else if (token != ')') {
             depth=0;
             while (token != EndOfInput && (depth > 0 || token != ')')) {
@@ -1184,11 +1184,11 @@ int TPpContext::MacroExpand(TPpToken* ppToken, bool expandUndef, bool newLineOka
             }
 
             if (token == EndOfInput) {
-                parseContext.ppError(loc, "End of input in macro", "macro expansion", GetAtomString(macroAtom));
+                parseContext.ppError(loc, "End of input in macro", "macro expansion", atomStrings.getString(macroAtom));
                 delete in;
                 return 0;
             }
-            parseContext.ppError(loc, "Too many args in macro", "macro expansion", GetAtomString(macroAtom));
+            parseContext.ppError(loc, "Too many args in macro", "macro expansion", atomStrings.getString(macroAtom));
         }
 
         // We need both expanded and non-expanded forms of the argument, for whether or

glslang/MachineIndependent/preprocessor/PpAtom.cpp

@@ -150,61 +150,12 @@ const struct {
 namespace glslang {
 
 //
-// Map an existing string to an atom.
-//
-// Return 0 if no existing string.
-//
-int TPpContext::LookUpString(const char* s)
-{
-    auto it = atomMap.find(s);
-    return it == atomMap.end() ? 0 : it->second;
-}
-
-//
-// Map a new or existing string to an atom, inventing a new atom if necessary.
-//
-int TPpContext::LookUpAddString(const char* s)
-{
-    int atom = LookUpString(s);
-    if (atom == 0) {
-        atom = nextAtom++;
-        AddAtomFixed(s, atom);
-    }
-
-    return atom;
-}
-
-//
-// Lookup up mapping of atom -> string.
-//
-const char* TPpContext::GetAtomString(int atom)
-{
-    if ((size_t)atom >= stringMap.size())
-        return "<bad token>";
-
-    const TString* atomString = stringMap[atom];
-
-    return atomString ? atomString->c_str() : "<bad token>";
-}
-
-//
-// Add mappings:
-//  - string -> atom
-//  - atom -> string
-//
-void TPpContext::AddAtomFixed(const char* s, int atom)
-{
-    auto it = atomMap.insert(std::pair<TString, int>(s, atom)).first;
-    if (stringMap.size() < (size_t)atom + 1)
-        stringMap.resize(atom + 100, 0);
-    stringMap[atom] = &it->first;
-}
-
-//
 // Initialize the atom table.
 //
-void TPpContext::InitAtomTable()
+TStringAtomMap::TStringAtomMap()
 {
+    badToken.assign("<bad token>");
+
     // Add single character tokens to the atom table:
     const char* s = "~!%^&*()-+=|,.<>/?;:[]{}#\\";
     char t[2];
@@ -212,13 +163,13 @@ void TPpContext::InitAtomTable()
     t[1] = '\0';
     while (*s) {
         t[0] = *s;
-        AddAtomFixed(t, s[0]);
+        addAtomFixed(t, s[0]);
         s++;
     }
 
     // Add multiple character scanner tokens :
     for (size_t ii = 0; ii < sizeof(tokens)/sizeof(tokens[0]); ii++)
-        AddAtomFixed(tokens[ii].str, tokens[ii].val);
+        addAtomFixed(tokens[ii].str, tokens[ii].val);
 
     nextAtom = PpAtomLast;
 }

glslang/MachineIndependent/preprocessor/PpContext.cpp

@@ -87,8 +87,6 @@ TPpContext::TPpContext(TParseContextBase& pc, const std::string& rootFileName, T
     rootFileName(rootFileName),
     currentSourceFile(rootFileName)
 {
-    InitAtomTable(); 
-
     ifdepth = 0;
     for (elsetracker = 0; elsetracker < maxIfNesting; elsetracker++)
         elseSeen[elsetracker] = false;

glslang/MachineIndependent/preprocessor/PpContext.h

@@ -115,6 +115,62 @@ public:
     char   name[MaxTokenLength + 1];
 };
 
+class TStringAtomMap {
+//
+// Implementation is in PpAtom.cpp
+//
+// Maintain a bi-directional mapping between relevant preprocessor strings and
+// "atoms" which a unique integers (small, contiguous, not hash-like) per string.
+//
+public:
+    TStringAtomMap();
+
+    // Map string -> atom.
+    // Return 0 if no existing string.
+    int getAtom(const char* s) const
+    {
+        auto it = atomMap.find(s);
+        return it == atomMap.end() ? 0 : it->second;
+    }
+
+    // Map a new or existing string -> atom, inventing a new atom if necessary.
+    int getAddAtom(const char* s)
+    {
+        int atom = getAtom(s);
+        if (atom == 0) {
+            atom = nextAtom++;
+            addAtomFixed(s, atom);
+        }
+        return atom;
+    }
+
+    // Map atom -> string.
+    const char* getString(int atom) const { return stringMap[atom]->c_str(); }
+
+protected:
+    TStringAtomMap(TStringAtomMap&);
+    TStringAtomMap& operator=(TStringAtomMap&);
+
+    TUnorderedMap<TString, int> atomMap;
+    TVector<const TString*> stringMap;    // these point into the TString in atomMap
+    int nextAtom;
+
+    // Bad source characters can lead to bad atoms, so gracefully handle those by
+    // pre-filling the table with them (to avoid if tests later).
+    TString badToken;
+
+    // Add bi-directional mappings:
+    //  - string -> atom
+    //  - atom -> string
+    void addAtomFixed(const char* s, int atom)
+    {
+        auto it = atomMap.insert(std::pair<TString, int>(s, atom)).first;
+        if (stringMap.size() < (size_t)atom + 1)
+            stringMap.resize(atom + 100, &badToken);
+        stringMap[atom] = &it->first;
+    }
+};
+
 class TInputScanner;
 
 // This class is the result of turning a huge pile of C code communicating through globals
@@ -196,6 +252,7 @@ protected:
     TPpContext(TPpContext&);
     TPpContext& operator=(TPpContext&);
 
+    TStringAtomMap atomStrings;
     char*   preamble;               // string to parse, all before line 1 of string 0, it is 0 if no preamble
     int     preambleLength;
     char**  strings;                // official strings of shader, starting a string 0 line 1
@@ -538,21 +595,6 @@ protected:
     std::string rootFileName;
     std::stack<TShader::Includer::IncludeResult*> includeStack;
     std::string currentSourceFile;
-
-    //
-    // From PpAtom.cpp
-    //
-    typedef TUnorderedMap<TString, int> TAtomMap;
-    typedef TVector<const TString*> TStringMap;
-
-    TAtomMap atomMap;
-    TStringMap stringMap;
-    int nextAtom;
-    void InitAtomTable();
-    void AddAtomFixed(const char* s, int atom);
-    int LookUpString(const char* s);
-    int LookUpAddString(const char* s);
-    const char* GetAtomString(int atom);
 };
 
 } // end namespace glslang

glslang/MachineIndependent/preprocessor/PpScanner.cpp

@@ -774,7 +774,7 @@ int TPpContext::tokenize(TPpToken& ppToken)
             parseContext.ppError(ppToken.loc, "character literals not supported", "\'", "");
             continue;
         default:
-            strcpy(ppToken.name, GetAtomString(token));
+            strcpy(ppToken.name, atomStrings.getString(token));
             break;
         }
 
@@ -836,8 +836,8 @@ int TPpContext::tokenPaste(int token, TPpToken& ppToken)
         case PpAtomAnd:
         case PpAtomOr:
         case PpAtomXor:
-            strcpy(ppToken.name, GetAtomString(resultToken));
-            strcpy(pastedPpToken.name, GetAtomString(token));
+            strcpy(ppToken.name, atomStrings.getString(resultToken));
+            strcpy(pastedPpToken.name, atomStrings.getString(token));
             break;
         default:
             parseContext.ppError(ppToken.loc, "not supported for these tokens", "##", "");
@@ -853,7 +853,7 @@ int TPpContext::tokenPaste(int token, TPpToken& ppToken)
 
         // correct the kind of token we are making, if needed (identifiers stay identifiers)
         if (resultToken != PpAtomIdentifier) {
-            int newToken = LookUpString(ppToken.name);
+            int newToken = atomStrings.getAtom(ppToken.name);
             if (newToken > 0)
                 resultToken = newToken;
             else