Unify declaration parsing code

Remove the unused identifierSymbol parameter from parseSingleDeclarator
and unify the ordering of parameters and the code style of different
declaration and declarator parsing functions. Some minor functional
changes to array size handling are done mainly to unify error message
generation. There's soon going to be more of these functions, so it's
good to be systematic.

TEST=angle_unittests, WebGL conformance tests
BUG=angleproject:941

Change-Id: I03b0220de93ca5719fdb7c1790a5999b8cb5b225
Reviewed-on: https://chromium-review.googlesource.com/265202
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index e9b392e..5a6b60a 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -673,9 +673,10 @@
 {
     TIntermConstantUnion* constant = expr->getAsConstantUnion();
 
-    if (constant == 0 || !constant->isScalarInt())
+    if (constant == nullptr || !constant->isScalarInt())
     {
         error(line, "array size must be a constant integer expression", "");
+        size = 1;
         return true;
     }
 
@@ -1076,8 +1077,9 @@
 // Returns true on error, false if no error
 //
 bool TParseContext::executeInitializer(const TSourceLoc &line, const TString &identifier, TPublicType &pType,
-                                       TIntermTyped *initializer, TIntermNode *&intermNode)
+                                       TIntermTyped *initializer, TIntermNode **intermNode)
 {
+    ASSERT(intermNode != nullptr);
     TType type = TType(pType);
 
     TVariable *variable = nullptr;
@@ -1130,16 +1132,22 @@
             return true;
         }
     }
- 
-    if (qualifier != EvqConst) {
-        TIntermSymbol* intermSymbol = intermediate.addSymbol(variable->getUniqueId(), variable->getName(), variable->getType(), line);
-        intermNode = createAssign(EOpInitialize, intermSymbol, initializer, line);
-        if (intermNode == 0) {
+
+    if (qualifier != EvqConst)
+    {
+        TIntermSymbol *intermSymbol = intermediate.addSymbol(variable->getUniqueId(), variable->getName(),
+                                                             variable->getType(), line);
+        *intermNode = createAssign(EOpInitialize, intermSymbol, initializer, line);
+        if (*intermNode == nullptr)
+        {
             assignError(line, "=", intermSymbol->getCompleteString(), initializer->getCompleteString());
             return true;
         }
-    } else 
-        intermNode = 0;
+    }
+    else
+    {
+        *intermNode = nullptr;
+    }
 
     return false;
 }
@@ -1236,7 +1244,6 @@
                                                         const TString &identifier)
 {
     TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, TType(publicType), identifierOrTypeLocation);
-    TIntermAggregate *aggregate = intermediate.makeAggregate(symbol, identifierOrTypeLocation);
 
     mDeferredSingleDeclarationErrorCheck = (identifier == "");
 
@@ -1253,15 +1260,17 @@
             recover();
 
         if (variable && symbol)
-        {
             symbol->setId(variable->getUniqueId());
-        }
     }
 
-    return aggregate;
+    return intermediate.makeAggregate(symbol, identifierOrTypeLocation);
 }
 
-TIntermAggregate* TParseContext::parseSingleArrayDeclaration(TPublicType &publicType, const TSourceLoc& identifierLocation, const TString &identifier, const TSourceLoc& indexLocation, TIntermTyped *indexExpression)
+TIntermAggregate *TParseContext::parseSingleArrayDeclaration(TPublicType &publicType,
+                                                             const TSourceLoc &identifierLocation,
+                                                             const TString &identifier,
+                                                             const TSourceLoc &indexLocation,
+                                                             TIntermTyped *indexExpression)
 {
     mDeferredSingleDeclarationErrorCheck = false;
 
@@ -1283,49 +1292,48 @@
     {
         recover();
     }
-    else
-    {
-        arrayType.setArraySize(size);
-    }
-
-    TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, arrayType, identifierLocation);
-    TIntermAggregate* aggregate = intermediate.makeAggregate(symbol, identifierLocation);
+    // Make the type an array even if size check failed.
+    // This ensures useless error messages regarding the variable's non-arrayness won't follow.
+    arrayType.setArraySize(size);
 
     TVariable *variable = nullptr;
     if (!declareVariable(identifierLocation, identifier, arrayType, &variable))
         recover();
 
+    TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, arrayType, identifierLocation);
     if (variable && symbol)
-    {
         symbol->setId(variable->getUniqueId());
-    }
 
-    return aggregate;
+    return intermediate.makeAggregate(symbol, identifierLocation);
 }
 
-TIntermAggregate* TParseContext::parseSingleInitDeclaration(TPublicType &publicType, const TSourceLoc& identifierLocation, const TString &identifier, const TSourceLoc& initLocation, TIntermTyped *initializer)
+TIntermAggregate *TParseContext::parseSingleInitDeclaration(TPublicType &publicType,
+                                                            const TSourceLoc &identifierLocation,
+                                                            const TString &identifier,
+                                                            const TSourceLoc &initLocation,
+                                                            TIntermTyped *initializer)
 {
     mDeferredSingleDeclarationErrorCheck = false;
 
     if (singleDeclarationErrorCheck(publicType, identifierLocation))
         recover();
 
-    TIntermNode* intermNode;
-    if (!executeInitializer(identifierLocation, identifier, publicType, initializer, intermNode))
+    TIntermNode *intermNode = nullptr;
+    if (!executeInitializer(identifierLocation, identifier, publicType, initializer, &intermNode))
     {
         //
         // Build intermediate representation
         //
-        return intermNode ? intermediate.makeAggregate(intermNode, initLocation) : NULL;
+        return intermNode ? intermediate.makeAggregate(intermNode, initLocation) : nullptr;
     }
     else
     {
         recover();
-        return NULL;
+        return nullptr;
     }
 }
 
-TIntermAggregate* TParseContext::parseInvariantDeclaration(const TSourceLoc &invariantLoc,
+TIntermAggregate *TParseContext::parseInvariantDeclaration(const TSourceLoc &invariantLoc,
                                                            const TSourceLoc &identifierLoc,
                                                            const TString *identifier,
                                                            const TSymbol *symbol)
@@ -1340,7 +1348,7 @@
     {
         error(identifierLoc, "undeclared identifier declared as invariant", identifier->c_str());
         recover();
-        return NULL;
+        return nullptr;
     }
     else
     {
@@ -1349,7 +1357,7 @@
         {
             error(identifierLoc, "identifier should not be declared as invariant", identifier->c_str());
             recover();
-            return NULL;
+            return nullptr;
         }
         symbolTable.addInvariantVarying(std::string(identifier->c_str()));
         const TVariable *variable = getNamedVariable(identifierLoc, identifier, symbol);
@@ -1364,7 +1372,8 @@
     }
 }
 
-TIntermAggregate* TParseContext::parseDeclarator(TPublicType &publicType, TIntermAggregate *aggregateDeclaration, TSymbol *identifierSymbol, const TSourceLoc& identifierLocation, const TString &identifier)
+TIntermAggregate *TParseContext::parseDeclarator(TPublicType &publicType, TIntermAggregate *aggregateDeclaration,
+                                                 const TSourceLoc &identifierLocation, const TString &identifier)
 {
     // If the declaration starting this declarator list was empty (example: int,), some checks were not performed.
     if (mDeferredSingleDeclarationErrorCheck)
@@ -1374,9 +1383,6 @@
         mDeferredSingleDeclarationErrorCheck = false;
     }
 
-    TIntermSymbol* symbol = intermediate.addSymbol(0, identifier, TType(publicType), identifierLocation);
-    TIntermAggregate* intermAggregate = intermediate.growAggregate(aggregateDeclaration, symbol, identifierLocation);
-
     if (locationDeclaratorListCheck(identifierLocation, publicType))
         recover();
 
@@ -1386,13 +1392,17 @@
     TVariable *variable = nullptr;
     if (!declareVariable(identifierLocation, identifier, TType(publicType), &variable))
         recover();
-    if (symbol && variable)
+
+    TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, TType(publicType), identifierLocation);
+    if (variable && symbol)
         symbol->setId(variable->getUniqueId());
 
-    return intermAggregate;
+    return intermediate.growAggregate(aggregateDeclaration, symbol, identifierLocation);
 }
 
-TIntermAggregate* TParseContext::parseArrayDeclarator(TPublicType &publicType, const TSourceLoc& identifierLocation, const TString &identifier, const TSourceLoc& arrayLocation, TIntermNode *declaratorList, TIntermTyped *indexExpression)
+TIntermAggregate *TParseContext::parseArrayDeclarator(TPublicType &publicType, TIntermAggregate *aggregateDeclaration,
+                                                      const TSourceLoc &identifierLocation, const TString &identifier,
+                                                      const TSourceLoc &arrayLocation, TIntermTyped *indexExpression)
 {
     // If the declaration starting this declarator list was empty (example: int,), some checks were not performed.
     if (mDeferredSingleDeclarationErrorCheck)
@@ -1414,27 +1424,31 @@
     }
     else
     {
+        TType arrayType = TType(publicType);
         int size;
         if (arraySizeErrorCheck(arrayLocation, indexExpression, size))
+        {
             recover();
-        TType arrayType = TType(publicType);
+        }
         arrayType.setArraySize(size);
+
         TVariable *variable = nullptr;
-        if (!declareVariable(arrayLocation, identifier, arrayType, &variable))
+        if (!declareVariable(identifierLocation, identifier, arrayType, &variable))
             recover();
 
         TIntermSymbol *symbol = intermediate.addSymbol(0, identifier, arrayType, identifierLocation);
         if (variable && symbol)
-        {
             symbol->setId(variable->getUniqueId());
-        }
-        return intermediate.growAggregate(declaratorList, symbol, identifierLocation);
+
+        return intermediate.growAggregate(aggregateDeclaration, symbol, identifierLocation);
     }
 
     return nullptr;
 }
 
-TIntermAggregate* TParseContext::parseInitDeclarator(TPublicType &publicType, TIntermAggregate *declaratorList, const TSourceLoc& identifierLocation, const TString &identifier, const TSourceLoc& initLocation, TIntermTyped *initializer)
+TIntermAggregate *TParseContext::parseInitDeclarator(TPublicType &publicType, TIntermAggregate *aggregateDeclaration,
+                                                     const TSourceLoc &identifierLocation, const TString &identifier,
+                                                     const TSourceLoc &initLocation, TIntermTyped *initializer)
 {
     // If the declaration starting this declarator list was empty (example: int,), some checks were not performed.
     if (mDeferredSingleDeclarationErrorCheck)
@@ -1447,25 +1461,25 @@
     if (locationDeclaratorListCheck(identifierLocation, publicType))
         recover();
 
-    TIntermNode* intermNode;
-    if (!executeInitializer(identifierLocation, identifier, publicType, initializer, intermNode))
+    TIntermNode *intermNode = nullptr;
+    if (!executeInitializer(identifierLocation, identifier, publicType, initializer, &intermNode))
     {
         //
         // build the intermediate representation
         //
         if (intermNode)
         {
-            return intermediate.growAggregate(declaratorList, intermNode, initLocation);
+            return intermediate.growAggregate(aggregateDeclaration, intermNode, initLocation);
         }
         else
         {
-            return declaratorList;
+            return aggregateDeclaration;
         }
     }
     else
     {
         recover();
-        return NULL;
+        return nullptr;
     }
 }