Clarify error checking function names in the GLSL parser

Most error checking functions in ParseContext used to follow a format
like <property>ErrorCheck. Sometimes this function would check that
the node/value would have <property>, sometimes it would check that
the node/value would not have it, which was confusing. Change most of
these functions to use a lot more descriptive names, which clearly
communicate what they are checking for.

Also includes a bit of refactoring in constructorErrorCheck(), so that
the function only checks for errors rather than also setting the type
of the constructor node.

Also make TType::arraySize unsigned, and return a sanitized size from
checkIsValidArraySize() instead of using an output parameter.

BUG=angleproject:911
TEST=angle_unittests

Change-Id: Id9767b8c79594ad3f782f801ea68eb96df721a31
Reviewed-on: https://chromium-review.googlesource.com/367070
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index b23ee52..75129d7 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -213,9 +213,9 @@
     error(line, " wrong operand types ", op, extraInfo.c_str());
 }
 
-void TParseContext::precisionErrorCheck(const TSourceLoc &line,
-                                        TPrecision precision,
-                                        TBasicType type)
+void TParseContext::checkPrecisionSpecified(const TSourceLoc &line,
+                                            TPrecision precision,
+                                            TBasicType type)
 {
     if (!mChecksPrecisionErrors)
         return;
@@ -247,7 +247,7 @@
 //
 // Returns true if the was an error.
 //
-bool TParseContext::lValueErrorCheck(const TSourceLoc &line, const char *op, TIntermTyped *node)
+bool TParseContext::checkCanBeLValue(const TSourceLoc &line, const char *op, TIntermTyped *node)
 {
     TIntermSymbol *symNode    = node->getAsSymbolNode();
     TIntermBinary *binaryNode = node->getAsBinaryNode();
@@ -262,9 +262,9 @@
             case EOpIndexIndirect:
             case EOpIndexDirectStruct:
             case EOpIndexDirectInterfaceBlock:
-                return lValueErrorCheck(line, op, binaryNode->getLeft());
+                return checkCanBeLValue(line, op, binaryNode->getLeft());
             case EOpVectorSwizzle:
-                errorReturn = lValueErrorCheck(line, op, binaryNode->getLeft());
+                errorReturn = checkCanBeLValue(line, op, binaryNode->getLeft());
                 if (!errorReturn)
                 {
                     int offset[4] = {0, 0, 0, 0};
@@ -403,7 +403,7 @@
 
 // Both test, and if necessary spit out an error, to see if the node is really
 // a constant.
-void TParseContext::constErrorCheck(TIntermTyped *node)
+void TParseContext::checkIsConst(TIntermTyped *node)
 {
     if (node->getQualifier() != EvqConst)
     {
@@ -413,7 +413,7 @@
 
 // Both test, and if necessary spit out an error, to see if the node is really
 // an integer.
-void TParseContext::integerErrorCheck(TIntermTyped *node, const char *token)
+void TParseContext::checkIsScalarInteger(TIntermTyped *node, const char *token)
 {
     if (!node->isScalarInt())
     {
@@ -423,9 +423,9 @@
 
 // Both test, and if necessary spit out an error, to see if we are currently
 // globally scoped.
-void TParseContext::globalErrorCheck(const TSourceLoc &line, bool global, const char *token)
+void TParseContext::checkIsAtGlobalLevel(const TSourceLoc &line, const char *token)
 {
-    if (!global)
+    if (!symbolTable.atGlobalLevel())
     {
         error(line, "only allowed at global scope", token);
     }
@@ -439,7 +439,7 @@
 //
 // Returns true if there was an error.
 //
-bool TParseContext::reservedErrorCheck(const TSourceLoc &line, const TString &identifier)
+bool TParseContext::checkIsNotReserved(const TSourceLoc &line, const TString &identifier)
 {
     static const char *reservedErrMsg = "reserved built-in name";
     if (!symbolTable.atBuiltInLevel())
@@ -487,14 +487,12 @@
 //
 // Returns true if there was an error in construction.
 //
-bool TParseContext::constructorErrorCheck(const TSourceLoc &line,
-                                          TIntermNode *argumentsNode,
-                                          TFunction &function,
-                                          TOperator op,
-                                          TType *type)
+bool TParseContext::checkConstructorArguments(const TSourceLoc &line,
+                                              TIntermNode *argumentsNode,
+                                              const TFunction &function,
+                                              TOperator op,
+                                              const TType &type)
 {
-    *type = function.getReturnType();
-
     bool constructingMatrix = false;
     switch (op)
     {
@@ -520,7 +518,6 @@
     //
 
     size_t size         = 0;
-    bool constType      = true;
     bool full           = false;
     bool overFull       = false;
     bool matrixInMatrix = false;
@@ -534,24 +531,17 @@
             matrixInMatrix = true;
         if (full)
             overFull = true;
-        if (op != EOpConstructStruct && !type->isArray() && size >= type->getObjectSize())
+        if (op != EOpConstructStruct && !type.isArray() && size >= type.getObjectSize())
             full = true;
-        if (param.type->getQualifier() != EvqConst)
-            constType = false;
         if (param.type->isArray())
             arrayArg = true;
     }
 
-    if (constType)
-        type->setQualifier(EvqConst);
-
-    if (type->isArray())
+    if (type.isArray())
     {
-        if (type->isUnsizedArray())
-        {
-            type->setArraySize(static_cast<int>(function.getParamCount()));
-        }
-        else if (static_cast<size_t>(type->getArraySize()) != function.getParamCount())
+        // The size of an unsized constructor should already have been determined.
+        ASSERT(!type.isUnsizedArray());
+        if (static_cast<size_t>(type.getArraySize()) != function.getParamCount())
         {
             error(line, "array constructor needs one argument per array element", "constructor");
             return true;
@@ -564,7 +554,7 @@
         return true;
     }
 
-    if (matrixInMatrix && !type->isArray())
+    if (matrixInMatrix && !type.isArray())
     {
         if (function.getParamCount() != 1)
         {
@@ -580,8 +570,8 @@
         return true;
     }
 
-    if (op == EOpConstructStruct && !type->isArray() &&
-        type->getStruct()->fields().size() != function.getParamCount())
+    if (op == EOpConstructStruct && !type.isArray() &&
+        type.getStruct()->fields().size() != function.getParamCount())
     {
         error(line,
               "Number of constructor parameters does not match the number of structure fields",
@@ -589,10 +579,10 @@
         return true;
     }
 
-    if (!type->isMatrix() || !matrixInMatrix)
+    if (!type.isMatrix() || !matrixInMatrix)
     {
-        if ((op != EOpConstructStruct && size != 1 && size < type->getObjectSize()) ||
-            (op == EOpConstructStruct && size < type->getObjectSize()))
+        if ((op != EOpConstructStruct && size != 1 && size < type.getObjectSize()) ||
+            (op == EOpConstructStruct && size < type.getObjectSize()))
         {
             error(line, "not enough data provided for construction", "constructor");
             return true;
@@ -622,6 +612,38 @@
         }
     }
 
+    if (type.isArray())
+    {
+        // GLSL ES 3.00 section 5.4.4: Each argument must be the same type as the element type of
+        // the array.
+        for (TIntermNode *&argNode : *argumentsAgg->getSequence())
+        {
+            const TType &argType = argNode->getAsTyped()->getType();
+            // It has already been checked that the argument is not an array.
+            ASSERT(!argType.isArray());
+            if (!argType.sameElementType(type))
+            {
+                error(line, "Array constructor argument has an incorrect type", "Error");
+                return true;
+            }
+        }
+    }
+    else if (op == EOpConstructStruct)
+    {
+        const TFieldList &fields = type.getStruct()->fields();
+        TIntermSequence *args    = argumentsAgg->getSequence();
+
+        for (size_t i = 0; i < fields.size(); i++)
+        {
+            if (i >= args->size() || (*args)[i]->getAsTyped()->getType() != *fields[i]->type())
+            {
+                error(line, "Structure constructor arguments do not match structure fields",
+                      "Error");
+                return true;
+            }
+        }
+    }
+
     return false;
 }
 
@@ -630,7 +652,7 @@
 //
 // returns true in case of an error
 //
-bool TParseContext::voidErrorCheck(const TSourceLoc &line,
+bool TParseContext::checkIsNonVoid(const TSourceLoc &line,
                                    const TString &identifier,
                                    const TBasicType &type)
 {
@@ -645,7 +667,7 @@
 
 // This function checks to see if the node (for the expression) contains a scalar boolean expression
 // or not.
-void TParseContext::boolErrorCheck(const TSourceLoc &line, const TIntermTyped *type)
+void TParseContext::checkIsScalarBool(const TSourceLoc &line, const TIntermTyped *type)
 {
     if (type->getBasicType() != EbtBool || type->isArray() || type->isMatrix() || type->isVector())
     {
@@ -655,7 +677,7 @@
 
 // This function checks to see if the node (for the expression) contains a scalar boolean expression
 // or not.
-void TParseContext::boolErrorCheck(const TSourceLoc &line, const TPublicType &pType)
+void TParseContext::checkIsScalarBool(const TSourceLoc &line, const TPublicType &pType)
 {
     if (pType.type != EbtBool || pType.isAggregate())
     {
@@ -663,7 +685,7 @@
     }
 }
 
-bool TParseContext::samplerErrorCheck(const TSourceLoc &line,
+bool TParseContext::checkIsNotSampler(const TSourceLoc &line,
                                       const TPublicType &pType,
                                       const char *reason)
 {
@@ -688,7 +710,8 @@
     return false;
 }
 
-void TParseContext::locationDeclaratorListCheck(const TSourceLoc &line, const TPublicType &pType)
+void TParseContext::checkDeclaratorLocationIsNotSpecified(const TSourceLoc &line,
+                                                          const TPublicType &pType)
 {
     if (pType.layoutQualifier.location != -1)
     {
@@ -697,9 +720,19 @@
     }
 }
 
-void TParseContext::parameterSamplerErrorCheck(const TSourceLoc &line,
-                                               TQualifier qualifier,
-                                               const TType &type)
+void TParseContext::checkLocationIsNotSpecified(const TSourceLoc &location,
+                                                const TLayoutQualifier &layoutQualifier)
+{
+    if (layoutQualifier.location != -1)
+    {
+        error(location, "invalid layout qualifier:", "location",
+              "only valid on program inputs and outputs");
+    }
+}
+
+void TParseContext::checkOutParameterIsNotSampler(const TSourceLoc &line,
+                                                  TQualifier qualifier,
+                                                  const TType &type)
 {
     if ((qualifier == EvqOut || qualifier == EvqInOut) && type.getBasicType() != EbtStruct &&
         IsSampler(type.getBasicType()))
@@ -727,7 +760,7 @@
 }
 
 // Do size checking for an array type's size.
-void TParseContext::arraySizeErrorCheck(const TSourceLoc &line, TIntermTyped *expr, int &size)
+unsigned int TParseContext::checkIsValidArraySize(const TSourceLoc &line, TIntermTyped *expr)
 {
     TIntermConstantUnion *constant = expr->getAsConstantUnion();
 
@@ -737,36 +770,32 @@
     if (expr->getQualifier() != EvqConst || constant == nullptr || !constant->isScalarInt())
     {
         error(line, "array size must be a constant integer expression", "");
-        size = 1;
-        return;
+        return 1u;
     }
 
-    unsigned int unsignedSize = 0;
+    unsigned int size = 0u;
 
     if (constant->getBasicType() == EbtUInt)
     {
-        unsignedSize = constant->getUConst(0);
-        size         = static_cast<int>(unsignedSize);
+        size = constant->getUConst(0);
     }
     else
     {
-        size = constant->getIConst(0);
+        int signedSize = constant->getIConst(0);
 
-        if (size < 0)
+        if (signedSize < 0)
         {
             error(line, "array size must be non-negative", "");
-            size = 1;
-            return;
+            return 1u;
         }
 
-        unsignedSize = static_cast<unsigned int>(size);
+        size = static_cast<unsigned int>(signedSize);
     }
 
-    if (size == 0)
+    if (size == 0u)
     {
         error(line, "array size must be greater than zero", "");
-        size = 1;
-        return;
+        return 1u;
     }
 
     // The size of arrays is restricted here to prevent issues further down the
@@ -774,19 +803,20 @@
     // 4096 registers so this should be reasonable even for aggressively optimizable code.
     const unsigned int sizeLimit = 65536;
 
-    if (unsignedSize > sizeLimit)
+    if (size > sizeLimit)
     {
         error(line, "array size too large", "");
-        size = 1;
-        return;
+        return 1u;
     }
+
+    return size;
 }
 
 // See if this qualifier can be an array.
 //
 // Returns true if there is an error.
 //
-bool TParseContext::arrayQualifierErrorCheck(const TSourceLoc &line, const TPublicType &type)
+bool TParseContext::checkIsValidQualifierForArray(const TSourceLoc &line, const TPublicType &type)
 {
     if ((type.qualifier == EvqAttribute) || (type.qualifier == EvqVertexIn) ||
         (type.qualifier == EvqConst && mShaderVersion < 300))
@@ -803,7 +833,7 @@
 //
 // Returns true if there is an error.
 //
-bool TParseContext::arrayTypeErrorCheck(const TSourceLoc &line, const TPublicType &type)
+bool TParseContext::checkIsValidTypeForArray(const TSourceLoc &line, const TPublicType &type)
 {
     //
     // Can the type be an array?
@@ -827,9 +857,9 @@
 }
 
 // Enforce non-initializer type/qualifier rules.
-void TParseContext::nonInitErrorCheck(const TSourceLoc &line,
-                                      const TString &identifier,
-                                      TPublicType *type)
+void TParseContext::checkCanBeDeclaredWithoutInitializer(const TSourceLoc &line,
+                                                         const TString &identifier,
+                                                         TPublicType *type)
 {
     ASSERT(type != nullptr);
     if (type->qualifier == EvqConst)
@@ -870,18 +900,18 @@
 {
     ASSERT((*variable) == nullptr);
 
-    bool needsReservedErrorCheck = true;
+    bool needsReservedCheck = true;
 
     // gl_LastFragData may be redeclared with a new precision qualifier
     if (type.isArray() && identifier.compare(0, 15, "gl_LastFragData") == 0)
     {
         const TVariable *maxDrawBuffers = static_cast<const TVariable *>(
             symbolTable.findBuiltIn("gl_MaxDrawBuffers", mShaderVersion));
-        if (type.getArraySize() == maxDrawBuffers->getConstPointer()->getIConst())
+        if (static_cast<int>(type.getArraySize()) == maxDrawBuffers->getConstPointer()->getIConst())
         {
             if (TSymbol *builtInSymbol = symbolTable.findBuiltIn(identifier, mShaderVersion))
             {
-                needsReservedErrorCheck = extensionErrorCheck(line, builtInSymbol->getExtension());
+                needsReservedCheck = checkCanUseExtension(line, builtInSymbol->getExtension());
             }
         }
         else
@@ -892,7 +922,7 @@
         }
     }
 
-    if (needsReservedErrorCheck && reservedErrorCheck(line, identifier))
+    if (needsReservedCheck && checkIsNotReserved(line, identifier))
         return false;
 
     (*variable) = new TVariable(&identifier, type);
@@ -903,16 +933,16 @@
         return false;
     }
 
-    if (voidErrorCheck(line, identifier, type.getBasicType()))
+    if (checkIsNonVoid(line, identifier, type.getBasicType()))
         return false;
 
     return true;
 }
 
-void TParseContext::paramErrorCheck(const TSourceLoc &line,
-                                    TQualifier qualifier,
-                                    TQualifier paramQualifier,
-                                    TType *type)
+void TParseContext::checkIsParameterQualifierValid(const TSourceLoc &line,
+                                                   TQualifier qualifier,
+                                                   TQualifier paramQualifier,
+                                                   TType *type)
 {
     if (qualifier != EvqConst && qualifier != EvqTemporary)
     {
@@ -932,7 +962,7 @@
         type->setQualifier(paramQualifier);
 }
 
-bool TParseContext::extensionErrorCheck(const TSourceLoc &line, const TString &extension)
+bool TParseContext::checkCanUseExtension(const TSourceLoc &line, const TString &extension)
 {
     const TExtensionBehavior &extBehavior   = extensionBehavior();
     TExtensionBehavior::const_iterator iter = extBehavior.find(extension.c_str());
@@ -981,7 +1011,7 @@
     }
 
     if (publicType.qualifier != EvqUniform &&
-        samplerErrorCheck(identifierLocation, publicType, "samplers must be uniform"))
+        checkIsNotSampler(identifierLocation, publicType, "samplers must be uniform"))
     {
         return;
     }
@@ -1007,23 +1037,13 @@
 
     if (publicType.qualifier != EvqVertexIn && publicType.qualifier != EvqFragmentOut)
     {
-        layoutLocationErrorCheck(identifierLocation, publicType.layoutQualifier);
+        checkLocationIsNotSpecified(identifierLocation, publicType.layoutQualifier);
     }
 }
 
-void TParseContext::layoutLocationErrorCheck(const TSourceLoc &location,
-                                             const TLayoutQualifier &layoutQualifier)
-{
-    if (layoutQualifier.location != -1)
-    {
-        error(location, "invalid layout qualifier:", "location",
-              "only valid on program inputs and outputs");
-    }
-}
-
-void TParseContext::layoutSupportedErrorCheck(const TSourceLoc &location,
-                                              const TString &layoutQualifierName,
-                                              int versionRequired)
+void TParseContext::checkLayoutQualifierSupported(const TSourceLoc &location,
+                                                  const TString &layoutQualifierName,
+                                                  int versionRequired)
 {
 
     if (mShaderVersion < versionRequired)
@@ -1032,8 +1052,8 @@
     }
 }
 
-bool TParseContext::layoutWorkGroupSizeErrorCheck(const TSourceLoc &location,
-                                                  const TLayoutQualifier &layoutQualifier)
+bool TParseContext::checkWorkGroupSizeIsNotSpecified(const TSourceLoc &location,
+                                                     const TLayoutQualifier &layoutQualifier)
 {
     const TLocalSize &localSize = layoutQualifier.localSize;
     for (size_t i = 0u; i < localSize.size(); ++i)
@@ -1050,17 +1070,17 @@
 }
 
 void TParseContext::functionCallLValueErrorCheck(const TFunction *fnCandidate,
-                                                 TIntermAggregate *aggregate)
+                                                 TIntermAggregate *fnCall)
 {
     for (size_t i = 0; i < fnCandidate->getParamCount(); ++i)
     {
         TQualifier qual = fnCandidate->getParam(i).type->getQualifier();
         if (qual == EvqOut || qual == EvqInOut)
         {
-            TIntermTyped *node = (*(aggregate->getSequence()))[i]->getAsTyped();
-            if (lValueErrorCheck(node->getLine(), "assign", node))
+            TIntermTyped *argument = (*(fnCall->getSequence()))[i]->getAsTyped();
+            if (checkCanBeLValue(argument->getLine(), "assign", argument))
             {
-                error(node->getLine(),
+                error(argument->getLine(),
                       "Constant value cannot be passed for 'out' or 'inout' parameters.", "Error");
                 return;
             }
@@ -1068,8 +1088,8 @@
     }
 }
 
-void TParseContext::es3InvariantErrorCheck(const TQualifier qualifier,
-                                           const TSourceLoc &invariantLocation)
+void TParseContext::checkInvariantIsOutVariableES3(const TQualifier qualifier,
+                                                   const TSourceLoc &invariantLocation)
 {
     if (!sh::IsVaryingOut(qualifier) && qualifier != EvqFragmentOut)
     {
@@ -1154,7 +1174,7 @@
         if (symbolTable.findBuiltIn(variable->getName(), mShaderVersion) &&
             !variable->getExtension().empty())
         {
-            extensionErrorCheck(location, variable->getExtension());
+            checkCanUseExtension(location, variable->getExtension());
         }
 
         // Reject shaders using both gl_FragData and gl_FragColor
@@ -1386,7 +1406,7 @@
     returnType.invariant       = invariant;
     returnType.layoutQualifier = layoutQualifier;
 
-    layoutWorkGroupSizeErrorCheck(typeSpecifier.line, layoutQualifier);
+    checkWorkGroupSizeIsNotSpecified(typeSpecifier.line, layoutQualifier);
 
     if (mShaderVersion < 300)
     {
@@ -1412,11 +1432,11 @@
     {
         if (!layoutQualifier.isEmpty())
         {
-            globalErrorCheck(typeSpecifier.line, symbolTable.atGlobalLevel(), "layout");
+            checkIsAtGlobalLevel(typeSpecifier.line, "layout");
         }
         if (sh::IsVarying(qualifier) || qualifier == EvqVertexIn || qualifier == EvqFragmentOut)
         {
-            es3InputOutputTypeCheck(qualifier, typeSpecifier, typeSpecifier.line);
+            checkInputOutputTypeIsValidES3(qualifier, typeSpecifier, typeSpecifier.line);
         }
         if (qualifier == EvqComputeIn)
         {
@@ -1428,9 +1448,9 @@
     return returnType;
 }
 
-void TParseContext::es3InputOutputTypeCheck(const TQualifier qualifier,
-                                            const TPublicType &type,
-                                            const TSourceLoc &qualifierLocation)
+void TParseContext::checkInputOutputTypeIsValidES3(const TQualifier qualifier,
+                                                   const TPublicType &type,
+                                                   const TSourceLoc &qualifierLocation)
 {
     // An input/output variable can never be bool or a sampler. Samplers are checked elsewhere.
     if (type.type == EbtBool)
@@ -1525,7 +1545,7 @@
     {
         singleDeclarationErrorCheck(publicType, identifierOrTypeLocation);
 
-        nonInitErrorCheck(identifierOrTypeLocation, identifier, &publicType);
+        checkCanBeDeclaredWithoutInitializer(identifierOrTypeLocation, identifier, &publicType);
 
         TVariable *variable = nullptr;
         declareVariable(identifierOrTypeLocation, identifier, TType(publicType), &variable);
@@ -1547,17 +1567,16 @@
 
     singleDeclarationErrorCheck(publicType, identifierLocation);
 
-    nonInitErrorCheck(identifierLocation, identifier, &publicType);
+    checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &publicType);
 
-    if (!arrayTypeErrorCheck(indexLocation, publicType))
+    if (!checkIsValidTypeForArray(indexLocation, publicType))
     {
-        arrayQualifierErrorCheck(indexLocation, publicType);
+        checkIsValidQualifierForArray(indexLocation, publicType);
     }
 
     TType arrayType(publicType);
 
-    int size;
-    arraySizeErrorCheck(identifierLocation, indexExpression, size);
+    unsigned int size = checkIsValidArraySize(identifierLocation, indexExpression);
     // 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);
@@ -1609,19 +1628,19 @@
 
     singleDeclarationErrorCheck(publicType, identifierLocation);
 
-    if (!arrayTypeErrorCheck(indexLocation, publicType))
+    if (!checkIsValidTypeForArray(indexLocation, publicType))
     {
-        arrayQualifierErrorCheck(indexLocation, publicType);
+        checkIsValidQualifierForArray(indexLocation, publicType);
     }
 
     TPublicType arrayType(publicType);
 
-    int size = 0;
+    unsigned int size = 0u;
     // If indexExpression is nullptr, then the array will eventually get its size implicitly from
     // the initializer.
     if (indexExpression != nullptr)
     {
-        arraySizeErrorCheck(identifierLocation, indexExpression, size);
+        size = checkIsValidArraySize(identifierLocation, indexExpression);
     }
     // Make the type an array even if size check failed.
     // This ensures useless error messages regarding the variable's non-arrayness won't follow.
@@ -1645,7 +1664,7 @@
                                                            const TSymbol *symbol)
 {
     // invariant declaration
-    globalErrorCheck(invariantLoc, symbolTable.atGlobalLevel(), "invariant varying");
+    checkIsAtGlobalLevel(invariantLoc, "invariant varying");
 
     if (!symbol)
     {
@@ -1687,9 +1706,9 @@
         mDeferredSingleDeclarationErrorCheck = false;
     }
 
-    locationDeclaratorListCheck(identifierLocation, publicType);
+    checkDeclaratorLocationIsNotSpecified(identifierLocation, publicType);
 
-    nonInitErrorCheck(identifierLocation, identifier, &publicType);
+    checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &publicType);
 
     TVariable *variable = nullptr;
     declareVariable(identifierLocation, identifier, TType(publicType), &variable);
@@ -1717,16 +1736,15 @@
         mDeferredSingleDeclarationErrorCheck = false;
     }
 
-    locationDeclaratorListCheck(identifierLocation, publicType);
+    checkDeclaratorLocationIsNotSpecified(identifierLocation, publicType);
 
-    nonInitErrorCheck(identifierLocation, identifier, &publicType);
+    checkCanBeDeclaredWithoutInitializer(identifierLocation, identifier, &publicType);
 
-    if (!arrayTypeErrorCheck(arrayLocation, publicType) &&
-        !arrayQualifierErrorCheck(arrayLocation, publicType))
+    if (!checkIsValidTypeForArray(arrayLocation, publicType) &&
+        !checkIsValidQualifierForArray(arrayLocation, publicType))
     {
         TType arrayType = TType(publicType);
-        int size;
-        arraySizeErrorCheck(arrayLocation, indexExpression, size);
+        unsigned int size = checkIsValidArraySize(arrayLocation, indexExpression);
         arrayType.setArraySize(size);
 
         TVariable *variable = nullptr;
@@ -1758,7 +1776,7 @@
         mDeferredSingleDeclarationErrorCheck = false;
     }
 
-    locationDeclaratorListCheck(identifierLocation, publicType);
+    checkDeclaratorLocationIsNotSpecified(identifierLocation, publicType);
 
     TIntermNode *intermNode = nullptr;
     if (!executeInitializer(identifierLocation, identifier, publicType, initializer, &intermNode))
@@ -1798,21 +1816,21 @@
         mDeferredSingleDeclarationErrorCheck = false;
     }
 
-    locationDeclaratorListCheck(identifierLocation, publicType);
+    checkDeclaratorLocationIsNotSpecified(identifierLocation, publicType);
 
-    if (!arrayTypeErrorCheck(indexLocation, publicType))
+    if (!checkIsValidTypeForArray(indexLocation, publicType))
     {
-        arrayQualifierErrorCheck(indexLocation, publicType);
+        checkIsValidQualifierForArray(indexLocation, publicType);
     }
 
     TPublicType arrayType(publicType);
 
-    int size = 0;
+    unsigned int size = 0u;
     // If indexExpression is nullptr, then the array will eventually get its size implicitly from
     // the initializer.
     if (indexExpression != nullptr)
     {
-        arraySizeErrorCheck(identifierLocation, indexExpression, size);
+        size = checkIsValidArraySize(identifierLocation, indexExpression);
     }
     // Make the type an array even if size check failed.
     // This ensures useless error messages regarding the variable's non-arrayness won't follow.
@@ -1908,7 +1926,7 @@
     else
     {
 
-        if (layoutWorkGroupSizeErrorCheck(typeQualifier.line, typeQualifier.layoutQualifier))
+        if (checkWorkGroupSizeIsNotSpecified(typeQualifier.line, typeQualifier.layoutQualifier))
         {
             return;
         }
@@ -1927,7 +1945,7 @@
             return;
         }
 
-        layoutLocationErrorCheck(typeQualifier.line, typeQualifier.layoutQualifier);
+        checkLocationIsNotSpecified(typeQualifier.line, typeQualifier.layoutQualifier);
 
         if (layoutQualifier.matrixPacking != EmpUnspecified)
         {
@@ -2196,7 +2214,7 @@
         error(location, "no qualifiers allowed for function return", "layout");
     }
     // make sure a sampler is not involved as well...
-    samplerErrorCheck(location, type, "samplers can't be function return values");
+    checkIsNotSampler(location, type, "samplers can't be function return values");
     if (mShaderVersion < 300)
     {
         // Array return values are forbidden, but there's also no valid syntax for declaring array
@@ -2248,67 +2266,55 @@
 // This function is used to test for the correctness of the parameters passed to various constructor
 // functions and also convert them to the right datatype if it is allowed and required.
 //
-// Returns 0 for an error or the constructed node (aggregate or typed) for no error.
+// Returns a node to add to the tree regardless of if an error was generated or not.
 //
 TIntermTyped *TParseContext::addConstructor(TIntermNode *arguments,
-                                            TType *type,
                                             TOperator op,
                                             TFunction *fnCall,
                                             const TSourceLoc &line)
 {
+    TType type = fnCall->getReturnType();
+    if (type.isUnsizedArray())
+    {
+        type.setArraySize(static_cast<unsigned int>(fnCall->getParamCount()));
+    }
+    bool constType = true;
+    for (size_t i = 0; i < fnCall->getParamCount(); ++i)
+    {
+        const TConstParameter &param = fnCall->getParam(i);
+        if (param.type->getQualifier() != EvqConst)
+            constType = false;
+    }
+    if (constType)
+        type.setQualifier(EvqConst);
+
+    if (checkConstructorArguments(line, arguments, *fnCall, op, type))
+    {
+        TIntermTyped *dummyNode = intermediate.setAggregateOperator(nullptr, op, line);
+        dummyNode->setType(type);
+        return dummyNode;
+    }
     TIntermAggregate *constructor = arguments->getAsAggregate();
     ASSERT(constructor != nullptr);
 
-    if (type->isArray())
-    {
-        // GLSL ES 3.00 section 5.4.4: Each argument must be the same type as the element type of
-        // the array.
-        TIntermSequence *args = constructor->getSequence();
-        for (size_t i = 0; i < args->size(); i++)
-        {
-            const TType &argType = (*args)[i]->getAsTyped()->getType();
-            // It has already been checked that the argument is not an array.
-            ASSERT(!argType.isArray());
-            if (!argType.sameElementType(*type))
-            {
-                error(line, "Array constructor argument has an incorrect type", "Error");
-                return nullptr;
-            }
-        }
-    }
-    else if (op == EOpConstructStruct)
-    {
-        const TFieldList &fields = type->getStruct()->fields();
-        TIntermSequence *args    = constructor->getSequence();
-
-        for (size_t i = 0; i < fields.size(); i++)
-        {
-            if (i >= args->size() || (*args)[i]->getAsTyped()->getType() != *fields[i]->type())
-            {
-                error(line, "Structure constructor arguments do not match structure fields",
-                      "Error");
-
-                return 0;
-            }
-        }
-    }
-
     // Turn the argument list itself into a constructor
     constructor->setOp(op);
     constructor->setLine(line);
     ASSERT(constructor->isConstructor());
 
     // Need to set type before setPrecisionFromChildren() because bool doesn't have precision.
-    constructor->setType(*type);
+    constructor->setType(type);
 
     // Structs should not be precision qualified, the individual members may be.
     // Built-in types on the other hand should be precision qualified.
     if (op != EOpConstructStruct)
     {
         constructor->setPrecisionFromChildren();
-        type->setPrecision(constructor->getPrecision());
+        type.setPrecision(constructor->getPrecision());
     }
 
+    constructor->setType(type);
+
     TIntermTyped *constConstructor = intermediate.foldAggregateBuiltIn(constructor);
     if (constConstructor)
     {
@@ -2355,7 +2361,7 @@
                                                         TIntermConstantUnion *baseNode,
                                                         const TSourceLoc &location)
 {
-    ASSERT(index < baseNode->getArraySize());
+    ASSERT(index < static_cast<int>(baseNode->getArraySize()));
 
     TType arrayElementType = baseNode->getType();
     arrayElementType.clearArrayness();
@@ -2422,7 +2428,7 @@
                                                    TIntermTyped *arrayIndex,
                                                    const TSourceLoc &arrayIndexLine)
 {
-    reservedErrorCheck(nameLine, blockName);
+    checkIsNotReserved(nameLine, blockName);
 
     if (typeQualifier.qualifier != EvqUniform)
     {
@@ -2431,7 +2437,7 @@
     }
 
     TLayoutQualifier blockLayoutQualifier = typeQualifier.layoutQualifier;
-    layoutLocationErrorCheck(typeQualifier.line, blockLayoutQualifier);
+    checkLocationIsNotSpecified(typeQualifier.line, blockLayoutQualifier);
 
     if (blockLayoutQualifier.matrixPacking == EmpUnspecified)
     {
@@ -2443,7 +2449,7 @@
         blockLayoutQualifier.blockStorage = mDefaultBlockStorage;
     }
 
-    layoutWorkGroupSizeErrorCheck(nameLine, blockLayoutQualifier);
+    checkWorkGroupSizeIsNotSpecified(nameLine, blockLayoutQualifier);
 
     TSymbol *blockNameSymbol = new TInterfaceBlockName(&blockName);
     if (!symbolTable.declare(blockNameSymbol))
@@ -2476,7 +2482,7 @@
 
         // check layout qualifiers
         TLayoutQualifier fieldLayoutQualifier = fieldType->getLayoutQualifier();
-        layoutLocationErrorCheck(field->line(), fieldLayoutQualifier);
+        checkLocationIsNotSpecified(field->line(), fieldLayoutQualifier);
 
         if (fieldLayoutQualifier.blockStorage != EbsUnspecified)
         {
@@ -2499,10 +2505,10 @@
     }
 
     // add array index
-    int arraySize = 0;
-    if (arrayIndex != NULL)
+    unsigned int arraySize = 0;
+    if (arrayIndex != nullptr)
     {
-        arraySizeErrorCheck(arrayIndexLine, arrayIndex, arraySize);
+        arraySize = checkIsValidArraySize(arrayIndexLine, arrayIndex);
     }
 
     TInterfaceBlock *interfaceBlock =
@@ -2536,7 +2542,7 @@
     }
     else
     {
-        reservedErrorCheck(instanceLine, *instanceName);
+        checkIsNotReserved(instanceLine, *instanceName);
 
         // add a symbol for this interface block
         TVariable *instanceTypeDef = new TVariable(instanceName, interfaceBlockType, false);
@@ -3043,7 +3049,7 @@
                                    size_t index,
                                    TLocalSize *localSize)
 {
-    layoutSupportedErrorCheck(qualifierTypeLine, qualifierType, 310);
+    checkLayoutQualifierSupported(qualifierTypeLine, qualifierType, 310);
     if (intValue < 1)
     {
         std::string errorMessage = std::string(getLocalSizeString(index)) + " must be positive";
@@ -3194,9 +3200,9 @@
 TFieldList *TParseContext::addStructDeclaratorList(const TPublicType &typeSpecifier,
                                                    TFieldList *fieldList)
 {
-    voidErrorCheck(typeSpecifier.line, (*fieldList)[0]->name(), typeSpecifier.type);
+    checkIsNonVoid(typeSpecifier.line, (*fieldList)[0]->name(), typeSpecifier.type);
 
-    layoutWorkGroupSizeErrorCheck(typeSpecifier.line, typeSpecifier.layoutQualifier);
+    checkWorkGroupSizeIsNotSpecified(typeSpecifier.line, typeSpecifier.layoutQualifier);
 
     for (unsigned int i = 0; i < fieldList->size(); ++i)
     {
@@ -3214,10 +3220,10 @@
         // don't allow arrays of arrays
         if (type->isArray())
         {
-            arrayTypeErrorCheck(typeSpecifier.line, typeSpecifier);
+            checkIsValidTypeForArray(typeSpecifier.line, typeSpecifier);
         }
         if (typeSpecifier.array)
-            type->setArraySize(typeSpecifier.arraySize);
+            type->setArraySize(static_cast<unsigned int>(typeSpecifier.arraySize));
         if (typeSpecifier.userDef)
         {
             type->setStruct(typeSpecifier.userDef->getStruct());
@@ -3244,7 +3250,7 @@
 
     if (!structName->empty())
     {
-        reservedErrorCheck(nameLine, *structName);
+        checkIsNotReserved(nameLine, *structName);
         TVariable *userTypeDef = new TVariable(structName, *structureType, true);
         if (!symbolTable.declare(userTypeDef))
         {
@@ -3418,7 +3424,7 @@
                                                 TIntermTyped *child,
                                                 const TSourceLoc &loc)
 {
-    lValueErrorCheck(loc, GetOperatorString(op), child);
+    checkCanBeLValue(loc, GetOperatorString(op), child);
     return addUnaryMath(op, child, loc);
 }
 
@@ -3827,25 +3833,8 @@
     }
     else if (op != EOpNull)
     {
-        //
         // Then this should be a constructor.
-        // Don't go through the symbol table for constructors.
-        // Their parameters will be verified algorithmically.
-        //
-        TType type(EbtVoid, EbpUndefined);  // use this to get the type back
-        if (!constructorErrorCheck(loc, paramNode, *fnCall, op, &type))
-        {
-            //
-            // It's a constructor, of type 'type'.
-            //
-            callNode = addConstructor(paramNode, &type, op, fnCall, loc);
-        }
-
-        if (callNode == nullptr)
-        {
-            callNode = intermediate.setAggregateOperator(nullptr, op, loc);
-        }
-        callNode->setType(type);
+        callNode = addConstructor(paramNode, op, fnCall, loc);
     }
     else
     {
@@ -3862,7 +3851,7 @@
             //
             if (builtIn && !fnCandidate->getExtension().empty())
             {
-                extensionErrorCheck(loc, fnCandidate->getExtension());
+                checkCanUseExtension(loc, fnCandidate->getExtension());
             }
             op = fnCandidate->getBuiltInOp();
             if (builtIn && op != EOpNull)
@@ -3967,7 +3956,7 @@
                                                  TIntermTyped *falseBlock,
                                                  const TSourceLoc &loc)
 {
-    boolErrorCheck(loc, cond);
+    checkIsScalarBool(loc, cond);
 
     if (trueBlock->getType() != falseBlock->getType())
     {