Memory management hygiene: Use compare() instead of substr(), and put a few more…

README.txt

@@ -127,4 +127,20 @@ Basic Internal Operation
  -  Reduction of the tree to a linear byte-code style low-level intermediate
     representation is likely a good way to generate fully optimized code.
 
- -  There is currently some dead linker-type code still lying around.
+ -  There is currently some dead old-style linker-type code still lying around.
+
+ - Memory pool: parsing uses types derived from C++ std types, using a 
+   custom allocator that puts them in a memory pool.  This makes allocation
+   of individual container/contents just few cycles and deallocation free.
+   This pool is popped after the AST is made and processed.
+
+   The use is simple: if you are going to call "new", there are three cases:
+
+    - the object comes from the pool (its base class has the macro
+      POOL_ALLOCATOR_NEW_DELETE in it) and you do not have to call delete
+
+    - it is a TString, in which case call NewPoolTString(), which gets 
+      it from the pool, and there is no corresponding delete
+
+    - the object does not come from the pool, and you have to do normal
+      C++ memory management of what you 'new'

StandAlone/StandAlone.cpp

@@ -254,7 +254,7 @@ bool SetConfigFile(const std::string& name)
     if (name.size() < 5)
         return false;
 
-    if (name.substr(name.size() - 5, std::string::npos) == ".conf") {
+    if (name.compare(name.size() - 5, 5, ".conf") == 0) {
         ConfigFile = name;
         return true;
     }

glslang/Include/Common.h

@@ -91,7 +91,7 @@ namespace glslang {
 // Pool version of string.
 //
 typedef pool_allocator<char> TStringAllocator;
-typedef std::basic_string <char, std::char_traits<char>, TStringAllocator > TString;
+typedef std::basic_string <char, std::char_traits<char>, TStringAllocator> TString;
 inline TString* NewPoolTString(const char* s)
 {
 	void* memory = GetThreadPoolAllocator().allocate(sizeof(TString));
@@ -103,6 +103,8 @@ inline TString* NewPoolTString(const char* s)
 //
 template <class T> class TVector : public std::vector<T, pool_allocator<T> > {
 public:
+    POOL_ALLOCATOR_NEW_DELETE(GetThreadPoolAllocator())
+
     typedef typename std::vector<T, pool_allocator<T> >::size_type size_type;
     TVector() : std::vector<T, pool_allocator<T> >() {}
     TVector(const pool_allocator<T>& a) : std::vector<T, pool_allocator<T> >(a) {}

glslang/Include/ConstantUnion.h

@@ -421,19 +421,20 @@ class TConstUnionArray {
 public:
     POOL_ALLOCATOR_NEW_DELETE(GetThreadPoolAllocator())
 
-    TConstUnionArray() { unionArray = 0; }
+    TConstUnionArray() : unionArray(0) { }
+    virtual ~TConstUnionArray() { }
 
     explicit TConstUnionArray(int size)
     {
         if (size == 0)
             unionArray = 0;
         else
-            unionArray = new TVector<TConstUnion>(size);
+            unionArray =  new TConstUnionVector(size);
     }
     TConstUnionArray(const TConstUnionArray& a) : unionArray(a.unionArray) { }
     TConstUnionArray(const TConstUnionArray& a, int start, int size)
     {
-        unionArray = new TVector<TConstUnion>(size);
+        unionArray = new TConstUnionVector(size);
         for (int i = 0; i < size; ++i)
             (*unionArray)[i] = a[start + i];
     }
@@ -441,7 +442,7 @@ public:
     // Use this constructor for a smear operation
     TConstUnionArray(int size, const TConstUnion& val)
     {
-        unionArray = new TVector<TConstUnion>(size, val);
+        unionArray = new TConstUnionVector(size, val);
     }
 
     TConstUnion& operator[](int index) { return (*unionArray)[index]; }
@@ -462,7 +463,8 @@ public:
     bool empty() const { return unionArray == 0; }
 
 protected:
-    TVector<TConstUnion>* unionArray;
+    typedef TVector<TConstUnion> TConstUnionVector;
+    TConstUnionVector* unionArray;
 };
 
 } // end namespace glslang

glslang/Include/Types.h

@@ -150,20 +150,8 @@ struct TTypeLoc {
 };
 typedef TVector<TTypeLoc> TTypeList;
 
-inline TTypeList* NewPoolTTypeList()
-{
-	void* memory = GetThreadPoolAllocator().allocate(sizeof(TTypeList));
-	return new(memory) TTypeList;
-}
-
 typedef TVector<TString*> TIdentifierList;
 
-inline TIdentifierList* NewPoolTIdentifierList()
-{
-    void* memory = GetThreadPoolAllocator().allocate(sizeof(TIdentifierList));
-    return new(memory) TIdentifierList;
-}
-
 //
 // TODO: memory: TArraySizes can be replaced by something smaller.
 // Almost all arrays could be handled by two sizes each fitting
@@ -175,6 +163,8 @@ inline TIdentifierList* NewPoolTIdentifierList()
 // is used, it will be containing at least one size.
 
 struct TArraySizes {
+    POOL_ALLOCATOR_NEW_DELETE(GetThreadPoolAllocator())
+
     TArraySizes() : maxArraySize(0) { }
     int getSize() { return sizes.front(); }  // TArraySizes only exists if there is at least one dimension
     void setSize(int s) { sizes.push_back(s); }
@@ -185,12 +175,6 @@ protected:
     int maxArraySize; // for tracking maximum referenced index, before an explicit size is given
 };
 
-inline TArraySizes* NewPoolTArraySizes()
-{
-    void* memory = GetThreadPoolAllocator().allocate(sizeof(TArraySizes));
-    return new(memory) TArraySizes;
-}
-
 //
 // TPublicType (coming up after some dependent declarations)
 // is a workaround for a problem with the yacc stack,  It can't have
@@ -484,12 +468,12 @@ public:
         shallowCopy(copyOf);
 
         if (arraySizes) {
-            arraySizes = NewPoolTArraySizes();
+            arraySizes = new TArraySizes;
             *arraySizes = *copyOf.arraySizes;
         }
 
 		if (structure) {
-			structure = NewPoolTTypeList();
+			structure = new TTypeList;
     		TStructureMapIterator iter;
 			for (unsigned int i = 0; i < copyOf.structure->size(); ++i) {
 				TTypeLoc typeLoc;
@@ -604,7 +588,7 @@ public:
     void setArraySizes(TArraySizes* s)
     {
         // For when we don't want distinct types sharing the same descriptor.
-        arraySizes = NewPoolTArraySizes();
+        arraySizes = new TArraySizes;
         *arraySizes = *s;
     }
     void setArraySizes(const TType& type) { setArraySizes(type.arraySizes); }

glslang/MachineIndependent/Initialize.cpp

@@ -1692,7 +1692,7 @@ void IdentifyBuiltIns(int version, EProfile profile, EShLanguage language, TSymb
         if (version < FirstProfileVersion || profile == ECompatibilityProfile || (! ForwardCompatibility && profile != EEsProfile && version < 420)) {
             TPrecisionQualifier pq = profile == EEsProfile ? EpqMedium : EpqNone;
             TType fragData(EbtFloat, EvqFragColor, pq, 4);
-            TArraySizes* arraySizes = NewPoolTArraySizes();
+            TArraySizes* arraySizes = new TArraySizes;
             arraySizes->setSize(resources.maxDrawBuffers);
             fragData.setArraySizes(arraySizes);
             symbolTable.insert(*new TVariable(NewPoolTString("gl_FragData"), fragData));

glslang/MachineIndependent/ParseHelper.cpp

@@ -1204,7 +1204,7 @@ void TParseContext::globalCheck(TSourceLoc loc, bool global, const char* token)
 bool TParseContext::reservedErrorCheck(TSourceLoc loc, const TString& identifier)
 {
     if (! symbolTable.atBuiltInLevel()) {
-        if (identifier.substr(0, 3) == TString("gl_")) {
+        if (identifier.compare(0, 3, "gl_") == 0) {
             error(loc, "reserved built-in name", "gl_", "");
 
             return true;
@@ -1826,7 +1826,7 @@ void TParseContext::nonInitConstCheck(TSourceLoc loc, TString& identifier, TType
 //
 TVariable* TParseContext::redeclareBuiltin(TSourceLoc loc, const TString& identifier, bool& newDeclaration)
 {
-    if (profile == EEsProfile || identifier.substr(0, 3) != TString("gl_") || symbolTable.atBuiltInLevel())
+    if (profile == EEsProfile || identifier.compare(0, 3, "gl_") != 0 || symbolTable.atBuiltInLevel())
         return 0;
 
     // Potentially redeclaring a built-in variable...
@@ -2416,7 +2416,7 @@ void TParseContext::addBlock(TSourceLoc loc, TTypeList& typeList, const TString*
 
     // make an anonymous variable if no name was provided
     if (! instanceName)
-        instanceName = new TString("");
+        instanceName = NewPoolTString("");
 
     TVariable* variable = new TVariable(instanceName, blockType);
     if (! symbolTable.insert(*variable)) {

glslang/MachineIndependent/SymbolTable.cpp

@@ -198,7 +198,7 @@ void TSymbolTableLevel::relateToOperator(const char* name, TOperator op)
     while (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) {
+        if (parenAt != candidateName.npos && candidateName.compare(0, parenAt, name) == 0) {
             TFunction* function = (*candidate).second->getAsFunction();
             function->relateToOperator(op);
         } else

glslang/MachineIndependent/SymbolTable.h

@@ -85,7 +85,7 @@ public:
     virtual ~TSymbol() { }
 
     const TString& getName() const { return *name; }
-    void changeName(const char* buf) { name = new TString(buf); }
+    void changeName(const TString* newName) { name = newName; }
     virtual const TString& getMangledName() const { return getName(); }
     virtual TFunction* getAsFunction() { return 0; }
     virtual const TFunction* getAsFunction() const { return 0; }
@@ -257,7 +257,7 @@ public:
             // Give it a name and insert its members in the symbol table, pointing to the container.
             char buf[20];
             snprintf(buf, 20, "__anon__%d", anonId++);
-            symbol.changeName(buf);
+            symbol.changeName(NewPoolTString(buf));
 
             bool isOkay = true;
             const TTypeList& types = *symbol.getAsVariable()->getType().getStruct();
@@ -307,7 +307,7 @@ public:
         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)
+            if (parenAt != candidateName.npos && candidateName.compare(0, parenAt, name) == 0)
 
                 return true;
         }

glslang/MachineIndependent/glslang.y

@@ -800,7 +800,7 @@ block_structure
 
 identifier_list
     : COMMA IDENTIFIER {
-        $$ = NewPoolTIdentifierList();
+        $$ = new TIdentifierList;
         $$->push_back($2.string);
     }
     | identifier_list COMMA IDENTIFIER {
@@ -1291,12 +1291,12 @@ type_specifier
 array_specifier
     : LEFT_BRACKET RIGHT_BRACKET {
         $$.loc = $1.loc;
-        $$.arraySizes = NewPoolTArraySizes();
+        $$.arraySizes = new TArraySizes;
         $$.arraySizes->setSize(0);
     }
     | LEFT_BRACKET constant_expression RIGHT_BRACKET {
         $$.loc = $1.loc;
-        $$.arraySizes = NewPoolTArraySizes();
+        $$.arraySizes = new TArraySizes;
 
         int size;
         parseContext.arraySizeCheck($2->getLoc(), $2, size);
@@ -2051,7 +2051,7 @@ struct_declaration
 
 struct_declarator_list
     : struct_declarator {
-        $$ = NewPoolTTypeList();
+        $$ = new TTypeList;
         $$->push_back($1);
     }
     | struct_declarator_list COMMA struct_declarator {