Refactor array element type checks

src/compiler/translator/ParseContext.cpp

@@ -968,40 +968,42 @@ bool TParseContext::checkIsValidQualifierForArray(const TSourceLoc &line,
 }
 
 // See if this element type can be formed into an array.
-bool TParseContext::checkIsValidTypeForArray(const TSourceLoc &line, const TPublicType &elementType)
+bool TParseContext::checkArrayElementIsNotArray(const TSourceLoc &line,
+                                                const TPublicType &elementType)
 {
-    //
-    // Can the type be an array?
-    //
     if (elementType.array)
     {
         error(line, "cannot declare arrays of arrays",
               TType(elementType).getCompleteString().c_str());
         return false;
     }
+    return true;
+}
+
+// Check if this qualified element type can be formed into an array. This is only called when array
+// brackets are associated with an identifier in a declaration, like this:
+//   float a[2];
+// Similar checks are done in addFullySpecifiedType for array declarations where the array brackets
+// are associated with the type, like this:
+//   float[2] a;
+bool TParseContext::checkIsValidTypeAndQualifierForArray(const TSourceLoc &indexLocation,
+                                                         const TPublicType &elementType)
+{
+    if (!checkArrayElementIsNotArray(indexLocation, elementType))
+    {
+        return false;
+    }
     // In ESSL1.00 shaders, structs cannot be varying (section 4.3.5). This is checked elsewhere.
     // In ESSL3.00 shaders, struct inputs/outputs are allowed but not arrays of structs (section
     // 4.3.4).
     if (mShaderVersion >= 300 && elementType.getBasicType() == EbtStruct &&
         sh::IsVarying(elementType.qualifier))
     {
-        error(line, "cannot declare arrays of structs of this qualifier",
+        error(indexLocation, "cannot declare arrays of structs of this qualifier",
               TType(elementType).getCompleteString().c_str());
         return false;
     }
-
-    return true;
-}
-
-// Check if this qualified element type can be formed into an array.
-bool TParseContext::checkIsValidTypeAndQualifierForArray(const TSourceLoc &indexLocation,
-                                                         const TPublicType &elementType)
-{
-    if (checkIsValidTypeForArray(indexLocation, elementType))
-    {
-        return checkIsValidQualifierForArray(indexLocation, elementType);
-    }
-    return false;
+    return checkIsValidQualifierForArray(indexLocation, elementType);
 }
 
 // Enforce non-initializer type/qualifier rules.
@@ -3345,7 +3347,7 @@ TParameter TParseContext::parseParameterArrayDeclarator(const TString *identifie
                                                         const TSourceLoc &arrayLoc,
                                                         TPublicType *type)
 {
-    checkIsValidTypeForArray(arrayLoc, *type);
+    checkArrayElementIsNotArray(arrayLoc, *type);
     unsigned int size = checkIsValidArraySize(arrayLoc, arraySize);
     type->setArraySize(size);
     return parseParameterDeclarator(*type, identifier, identifierLoc);
@@ -4528,7 +4530,7 @@ TFieldList *TParseContext::addStructDeclaratorList(const TPublicType &typeSpecif
         // don't allow arrays of arrays
         if (!declaratorArraySizes.empty())
         {
-            checkIsValidTypeForArray(typeSpecifier.getLine(), typeSpecifier);
+            checkArrayElementIsNotArray(typeSpecifier.getLine(), typeSpecifier);
         }
 
         TType *type = (*declaratorList)[i]->type();

src/compiler/translator/ParseContext.h

@@ -125,7 +125,7 @@ class TParseContext : angle::NonCopyable
     // Returns a sanitized array size to use (the size is at least 1).
     unsigned int checkIsValidArraySize(const TSourceLoc &line, TIntermTyped *expr);
     bool checkIsValidQualifierForArray(const TSourceLoc &line, const TPublicType &elementQualifier);
-    bool checkIsValidTypeForArray(const TSourceLoc &line, const TPublicType &elementType);
+    bool checkArrayElementIsNotArray(const TSourceLoc &line, const TPublicType &elementType);
     bool checkIsNonVoid(const TSourceLoc &line, const TString &identifier, const TBasicType &type);
     bool checkIsScalarBool(const TSourceLoc &line, const TIntermTyped *type);
     void checkIsScalarBool(const TSourceLoc &line, const TPublicType &pType);

src/compiler/translator/glslang.y

@@ -943,11 +943,8 @@ type_specifier_no_prec
     }
     | type_specifier_nonarray LEFT_BRACKET constant_expression RIGHT_BRACKET {
         $$.initialize($1, (context->symbolTable.atGlobalLevel() ? EvqGlobal : EvqTemporary));
-        if (context->checkIsValidTypeForArray(@2, $$))
-        {
-            unsigned int size = context->checkIsValidArraySize(@2, $3);
-            $$.setArraySize(size);
-        }
+        unsigned int size = context->checkIsValidArraySize(@2, $3);
+        $$.setArraySize(size);
     }
     ;
 

src/compiler/translator/glslang_tab.cpp

@@ -757,20 +757,20 @@ static const yytype_uint16 yyrline[] =
      813,   817,   820,   823,   832,   837,   841,   844,   847,   850,
      853,   857,   860,   864,   867,   870,   873,   876,   879,   886,
      893,   896,   899,   905,   912,   915,   921,   924,   927,   930,
-     936,   939,   944,   955,   958,   961,   964,   967,   970,   974,
-     978,   982,   986,   990,   994,   998,  1002,  1006,  1010,  1014,
-    1018,  1022,  1026,  1030,  1034,  1038,  1042,  1046,  1050,  1054,
-    1060,  1063,  1066,  1069,  1072,  1075,  1078,  1081,  1084,  1087,
-    1090,  1093,  1096,  1099,  1102,  1105,  1108,  1111,  1114,  1121,
-    1127,  1133,  1136,  1139,  1142,  1145,  1148,  1151,  1154,  1157,
-    1160,  1163,  1166,  1169,  1172,  1175,  1183,  1183,  1186,  1186,
-    1192,  1195,  1201,  1204,  1211,  1215,  1221,  1224,  1230,  1234,
-    1238,  1239,  1245,  1246,  1247,  1248,  1249,  1250,  1251,  1255,
-    1256,  1256,  1256,  1263,  1264,  1268,  1268,  1269,  1269,  1274,
-    1277,  1284,  1288,  1295,  1296,  1300,  1306,  1310,  1317,  1317,
-    1324,  1327,  1333,  1337,  1343,  1343,  1348,  1348,  1352,  1352,
-    1360,  1363,  1369,  1372,  1378,  1382,  1389,  1392,  1395,  1398,
-    1401,  1409,  1415,  1421,  1424,  1430,  1430
+     936,   939,   944,   952,   955,   958,   961,   964,   967,   971,
+     975,   979,   983,   987,   991,   995,   999,  1003,  1007,  1011,
+    1015,  1019,  1023,  1027,  1031,  1035,  1039,  1043,  1047,  1051,
+    1057,  1060,  1063,  1066,  1069,  1072,  1075,  1078,  1081,  1084,
+    1087,  1090,  1093,  1096,  1099,  1102,  1105,  1108,  1111,  1118,
+    1124,  1130,  1133,  1136,  1139,  1142,  1145,  1148,  1151,  1154,
+    1157,  1160,  1163,  1166,  1169,  1172,  1180,  1180,  1183,  1183,
+    1189,  1192,  1198,  1201,  1208,  1212,  1218,  1221,  1227,  1231,
+    1235,  1236,  1242,  1243,  1244,  1245,  1246,  1247,  1248,  1252,
+    1253,  1253,  1253,  1260,  1261,  1265,  1265,  1266,  1266,  1271,
+    1274,  1281,  1285,  1292,  1293,  1297,  1303,  1307,  1314,  1314,
+    1321,  1324,  1330,  1334,  1340,  1340,  1345,  1345,  1349,  1349,
+    1357,  1360,  1366,  1369,  1375,  1379,  1386,  1389,  1392,  1395,
+    1398,  1406,  1412,  1418,  1421,  1427,  1427
 };
 #endif
 
@@ -3851,11 +3851,8 @@ yyreduce:
 
     {
         (yyval.interm.type).initialize((yyvsp[-3].interm.typeSpecifierNonArray), (context->symbolTable.atGlobalLevel() ? EvqGlobal : EvqTemporary));
-        if (context->checkIsValidTypeForArray((yylsp[-2]), (yyval.interm.type)))
-        {
-            unsigned int size = context->checkIsValidArraySize((yylsp[-2]), (yyvsp[-1].interm.intermTypedNode));
-            (yyval.interm.type).setArraySize(size);
-        }
+        unsigned int size = context->checkIsValidArraySize((yylsp[-2]), (yyvsp[-1].interm.intermTypedNode));
+        (yyval.interm.type).setArraySize(size);
     }
 
     break;