Refactor array element type checks
Remove checks that would never fail, and refactor the functions into
more self-contained checks. For example, it doesn't make sense to
check the qualifier from the part of the type that doesn't contain the
qualifier.
This prepares for adding the parsing of arrays of arrays.
BUG=angleproject:2125
TEST=angle_unittests
Change-Id: I1144bee35d2b04c7cb22e2bb7e17307298e35f8c
Reviewed-on: https://chromium-review.googlesource.com/629016
Reviewed-by: Corentin Wallez <cwallez@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 f422f04..a1c12f3 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -968,40 +968,42 @@
}
// 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 @@
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 @@
// don't allow arrays of arrays
if (!declaratorArraySizes.empty())
{
- checkIsValidTypeForArray(typeSpecifier.getLine(), typeSpecifier);
+ checkArrayElementIsNotArray(typeSpecifier.getLine(), typeSpecifier);
}
TType *type = (*declaratorList)[i]->type();
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 52edb4f..784de13 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -125,7 +125,7 @@
// 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);
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index f6c182b..8a5e6eb 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -943,11 +943,8 @@
}
| 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);
}
;
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index ed83499..794794b 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -757,20 +757,20 @@
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 @@
{
(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;