Simplify parsing struct field declarators

src/compiler.gypi

@@ -43,6 +43,8 @@
             'compiler/translator/Compiler.h',
             'compiler/translator/ConstantUnion.cpp',
             'compiler/translator/ConstantUnion.h',
+            'compiler/translator/Declarator.cpp',
+            'compiler/translator/Declarator.h',
             'compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h',
             'compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp',
             'compiler/translator/DeferGlobalInitializers.cpp',

src/compiler/translator/Declarator.cpp

+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Declarator.cpp:
+//   Declarator type for parsing structure field declarators.
+
+#include "compiler/translator/Declarator.h"
+
+namespace sh
+{
+
+TDeclarator::TDeclarator(const TString *name, const TSourceLoc &line)
+    : mName(name), mArraySizes(nullptr), mLine(line)
+{
+    ASSERT(mName);
+}
+
+TDeclarator::TDeclarator(const TString *name,
+                         const TVector<unsigned int> *arraySizes,
+                         const TSourceLoc &line)
+    : mName(name), mArraySizes(arraySizes), mLine(line)
+{
+    ASSERT(mName);
+    ASSERT(mArraySizes);
+}
+
+bool TDeclarator::isArray() const
+{
+    return mArraySizes != nullptr && mArraySizes->size() > 0;
+}
+
+}  // namespace sh

src/compiler/translator/Declarator.h

+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Declarator.h:
+//   Declarator type for parsing structure field declarators.
+
+#ifndef COMPILER_TRANSLATOR_DECLARATOR_H_
+#define COMPILER_TRANSLATOR_DECLARATOR_H_
+
+#include "compiler/translator/Common.h"
+
+namespace sh
+{
+
+// Declarator like "a[2][4]". Only used for parsing structure field declarators.
+class TDeclarator : angle::NonCopyable
+{
+  public:
+    POOL_ALLOCATOR_NEW_DELETE();
+    TDeclarator(const TString *name, const TSourceLoc &line);
+
+    TDeclarator(const TString *name,
+                const TVector<unsigned int> *arraySizes,
+                const TSourceLoc &line);
+
+    const TString *name() const { return mName; }
+
+    bool isArray() const;
+    const TVector<unsigned int> *arraySizes() const { return mArraySizes; }
+
+    const TSourceLoc &line() const { return mLine; }
+
+  private:
+    const TString *const mName;
+
+    // Outermost array size is stored at the end of the vector.
+    const TVector<unsigned int> *const mArraySizes;
+
+    const TSourceLoc mLine;
+};
+
+using TDeclaratorList = TVector<TDeclarator *>;
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_DECLARATOR_H_

src/compiler/translator/Initialize.cpp

@@ -741,7 +741,7 @@ void InsertBuiltInFunctions(sh::GLenum type,
     //
     // Depth range in window coordinates
     //
-    TFieldList *fields       = NewPoolTFieldList();
+    TFieldList *fields       = new TFieldList();
     TSourceLoc zeroSourceLoc = {0, 0, 0, 0};
     auto highpFloat1         = new TType(EbtFloat, EbpHigh, EvqGlobal, 1);
     TField *near             = new TField(highpFloat1, NewPoolTString("near"), zeroSourceLoc);
@@ -1014,7 +1014,7 @@ void IdentifyBuiltIns(sh::GLenum type,
             const TString *glPerVertexString = NewPoolTString("gl_PerVertex");
             symbolTable.insertInterfaceBlockNameExt(ESSL3_1_BUILTINS, extension, glPerVertexString);
 
-            TFieldList *fieldList    = NewPoolTFieldList();
+            TFieldList *fieldList    = new TFieldList();
             TSourceLoc zeroSourceLoc = {0, 0, 0, 0};
             TField *glPositionField  = new TField(new TType(EbtFloat, EbpHigh, EvqPosition, 4),
                                                  NewPoolTString("gl_Position"), zeroSourceLoc);

src/compiler/translator/ParseContext.cpp

@@ -12,6 +12,7 @@
 #include "common/mathutil.h"
 #include "compiler/preprocessor/SourceLocation.h"
 #include "compiler/translator/Cache.h"
+#include "compiler/translator/Declarator.h"
 #include "compiler/translator/IntermNode_util.h"
 #include "compiler/translator/ValidateGlobalInitializer.h"
 #include "compiler/translator/ValidateSwitch.h"
@@ -4674,24 +4675,18 @@ TLayoutQualifier TParseContext::joinLayoutQualifiers(TLayoutQualifier leftQualif
                                     mDiagnostics);
 }
 
-TField *TParseContext::parseStructDeclarator(TString *identifier, const TSourceLoc &loc)
+TDeclarator *TParseContext::parseStructDeclarator(const TString *identifier, const TSourceLoc &loc)
 {
     checkIsNotReserved(loc, *identifier);
-    TType *type = new TType(EbtVoid, EbpUndefined);
-    return new TField(type, identifier, loc);
+    return new TDeclarator(identifier, loc);
 }
 
-TField *TParseContext::parseStructArrayDeclarator(TString *identifier,
-                                                  const TSourceLoc &loc,
-                                                  const TVector<unsigned int> &arraySizes,
-                                                  const TSourceLoc &arraySizeLoc)
+TDeclarator *TParseContext::parseStructArrayDeclarator(const TString *identifier,
+                                                       const TSourceLoc &loc,
+                                                       const TVector<unsigned int> *arraySizes)
 {
     checkIsNotReserved(loc, *identifier);
-
-    TType *type       = new TType(EbtVoid, EbpUndefined);
-    type->makeArrays(arraySizes);
-
-    return new TField(type, identifier, loc);
+    return new TDeclarator(identifier, arraySizes, loc);
 }
 
 void TParseContext::checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
@@ -4735,7 +4730,7 @@ TFieldList *TParseContext::combineStructFieldLists(TFieldList *processedFields,
 TFieldList *TParseContext::addStructDeclaratorListWithQualifiers(
     const TTypeQualifierBuilder &typeQualifierBuilder,
     TPublicType *typeSpecifier,
-    TFieldList *fieldList)
+    const TDeclaratorList *declaratorList)
 {
     TTypeQualifier typeQualifier = typeQualifierBuilder.getVariableTypeQualifier(mDiagnostics);
 
@@ -4747,44 +4742,38 @@ TFieldList *TParseContext::addStructDeclaratorListWithQualifiers(
     {
         typeSpecifier->precision = typeQualifier.precision;
     }
-    return addStructDeclaratorList(*typeSpecifier, fieldList);
+    return addStructDeclaratorList(*typeSpecifier, declaratorList);
 }
 
 TFieldList *TParseContext::addStructDeclaratorList(const TPublicType &typeSpecifier,
-                                                   TFieldList *declaratorList)
+                                                   const TDeclaratorList *declaratorList)
 {
     checkPrecisionSpecified(typeSpecifier.getLine(), typeSpecifier.precision,
                             typeSpecifier.getBasicType());
 
-    checkIsNonVoid(typeSpecifier.getLine(), (*declaratorList)[0]->name(),
+    checkIsNonVoid(typeSpecifier.getLine(), *(*declaratorList)[0]->name(),
                    typeSpecifier.getBasicType());
 
     checkWorkGroupSizeIsNotSpecified(typeSpecifier.getLine(), typeSpecifier.layoutQualifier);
 
-    for (TField *declarator : *declaratorList)
+    TFieldList *fieldList = new TFieldList();
+
+    for (const TDeclarator *declarator : *declaratorList)
     {
-        // Don't allow arrays of arrays in ESSL < 3.10.
-        if (declarator->type()->isArray())
+        TType *type = new TType(typeSpecifier);
+        if (declarator->isArray())
         {
+            // Don't allow arrays of arrays in ESSL < 3.10.
             checkArrayElementIsNotArray(typeSpecifier.getLine(), typeSpecifier);
+            type->makeArrays(*declarator->arraySizes());
         }
 
-        auto *declaratorArraySizes = declarator->type()->getArraySizes();
-
-        TType *type = declarator->type();
-        *type       = TType(typeSpecifier);
-        if (declaratorArraySizes != nullptr)
-        {
-            for (unsigned int arraySize : *declaratorArraySizes)
-            {
-                type->makeArray(arraySize);
-            }
-        }
-
-        checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *declarator);
+        TField *field = new TField(type, declarator->name(), declarator->line());
+        checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *field);
+        fieldList->push_back(field);
     }
 
-    return declaratorList;
+    return fieldList;
 }
 
 TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,

src/compiler/translator/ParseContext.h

@@ -6,12 +6,13 @@
 #ifndef COMPILER_TRANSLATOR_PARSECONTEXT_H_
 #define COMPILER_TRANSLATOR_PARSECONTEXT_H_
 
+#include "compiler/preprocessor/Preprocessor.h"
 #include "compiler/translator/Compiler.h"
+#include "compiler/translator/Declarator.h"
 #include "compiler/translator/Diagnostics.h"
 #include "compiler/translator/DirectiveHandler.h"
-#include "compiler/translator/SymbolTable.h"
 #include "compiler/translator/QualifierTypes.h"
-#include "compiler/preprocessor/Preprocessor.h"
+#include "compiler/translator/SymbolTable.h"
 
 namespace sh
 {
@@ -311,11 +312,10 @@ class TParseContext : angle::NonCopyable
                                               const TSourceLoc &fieldLocation);
 
     // Parse declarator for a single field
-    TField *parseStructDeclarator(TString *identifier, const TSourceLoc &loc);
-    TField *parseStructArrayDeclarator(TString *identifier,
-                                       const TSourceLoc &loc,
-                                       const TVector<unsigned int> &arraySizes,
-                                       const TSourceLoc &arraySizeLoc);
+    TDeclarator *parseStructDeclarator(const TString *identifier, const TSourceLoc &loc);
+    TDeclarator *parseStructArrayDeclarator(const TString *identifier,
+                                            const TSourceLoc &loc,
+                                            const TVector<unsigned int> *arraySizes);
 
     void checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
                                             const TFieldList::const_iterator end,
@@ -328,8 +328,9 @@ class TParseContext : angle::NonCopyable
     TFieldList *addStructDeclaratorListWithQualifiers(
         const TTypeQualifierBuilder &typeQualifierBuilder,
         TPublicType *typeSpecifier,
-        TFieldList *fieldList);
-    TFieldList *addStructDeclaratorList(const TPublicType &typeSpecifier, TFieldList *fieldList);
+        const TDeclaratorList *declaratorList);
+    TFieldList *addStructDeclaratorList(const TPublicType &typeSpecifier,
+                                        const TDeclaratorList *declaratorList);
     TTypeSpecifierNonArray addStructure(const TSourceLoc &structLine,
                                         const TSourceLoc &nameLine,
                                         const TString *structName,

src/compiler/translator/Types.h

@@ -28,31 +28,26 @@ class TField : angle::NonCopyable
 {
   public:
     POOL_ALLOCATOR_NEW_DELETE();
-    TField(TType *type, TString *name, const TSourceLoc &line)
+    TField(TType *type, const TString *name, const TSourceLoc &line)
         : mType(type), mName(name), mLine(line)
     {
+        ASSERT(mName);
     }
 
     // TODO(alokp): We should only return const type.
     // Fix it by tweaking grammar.
     TType *type() { return mType; }
     const TType *type() const { return mType; }
-
     const TString &name() const { return *mName; }
     const TSourceLoc &line() const { return mLine; }
 
   private:
     TType *mType;
-    TString *mName;
-    TSourceLoc mLine;
+    const TString *mName;
+    const TSourceLoc mLine;
 };
 
 typedef TVector<TField *> TFieldList;
-inline TFieldList *NewPoolTFieldList()
-{
-    void *memory = GetGlobalPoolAllocator()->allocate(sizeof(TFieldList));
-    return new (memory) TFieldList;
-}
 
 class TFieldListCollection : angle::NonCopyable
 {

src/compiler/translator/glslang.y

@@ -40,6 +40,7 @@ WHICH GENERATES THE GLSL ES PARSER (glslang_tab.cpp AND glslang_tab.h).
 
 #include "angle_gl.h"
 #include "compiler/translator/Cache.h"
+#include "compiler/translator/Declarator.h"
 #include "compiler/translator/SymbolTable.h"
 #include "compiler/translator/ParseContext.h"
 #include "GLSLANG/ShaderLang.h"
@@ -94,7 +95,8 @@ using namespace sh;
             TQualifier qualifier;
             TFunction *function;
             TParameter param;
-            TField *field;
+            TDeclarator *declarator;
+            TDeclaratorList *declaratorList;
             TFieldList *fieldList;
             TQualifierWrapperBase *qualifierWrapper;
             TTypeQualifierBuilder *typeQualifierBuilder;
@@ -232,8 +234,9 @@ extern void yyerror(YYLTYPE* yylloc, TParseContext* context, void *scanner, cons
 
 %type <interm.typeSpecifierNonArray> type_specifier_nonarray struct_specifier
 %type <interm.type> type_specifier_no_prec
-%type <interm.field> struct_declarator
-%type <interm.fieldList> struct_declarator_list struct_declaration struct_declaration_list
+%type <interm.declarator> struct_declarator
+%type <interm.declaratorList> struct_declarator_list
+%type <interm.fieldList> struct_declaration struct_declaration_list
 %type <interm.function> function_header function_declarator function_identifier
 %type <interm.function> function_header_with_parameters function_call_header
 %type <interm> function_call_header_with_parameters function_call_header_no_parameters function_call_generic function_prototype
@@ -1229,7 +1232,7 @@ struct_declaration
 
 struct_declarator_list
     : struct_declarator {
-        $$ = NewPoolTFieldList();
+        $$ = new TDeclaratorList();
         $$->push_back($1);
     }
     | struct_declarator_list COMMA struct_declarator {
@@ -1242,7 +1245,7 @@ struct_declarator
         $$ = context->parseStructDeclarator($1.string, @1);
     }
     | identifier array_specifier {
-        $$ = context->parseStructArrayDeclarator($1.string, @1, *($2), @2);
+        $$ = context->parseStructArrayDeclarator($1.string, @1, $2);
     }
     ;
 

src/compiler/translator/glslang_tab.cpp

@@ -90,6 +90,7 @@
 
 #include "angle_gl.h"
 #include "compiler/translator/Cache.h"
+#include "compiler/translator/Declarator.h"
 #include "compiler/translator/SymbolTable.h"
 #include "compiler/translator/ParseContext.h"
 #include "GLSLANG/ShaderLang.h"
@@ -335,7 +336,8 @@ union YYSTYPE
             TQualifier qualifier;
             TFunction *function;
             TParameter param;
-            TField *field;
+            TDeclarator *declarator;
+            TDeclaratorList *declaratorList;
             TFieldList *fieldList;
             TQualifierWrapperBase *qualifierWrapper;
             TTypeQualifierBuilder *typeQualifierBuilder;
@@ -742,36 +744,36 @@ static const yytype_uint8 yytranslate[] =
   /* YYRLINE[YYN] -- Source line where rule number YYN was defined.  */
 static const yytype_uint16 yyrline[] =
 {
-       0,   248,   248,   249,   252,   262,   265,   270,   275,   280,
-     285,   294,   300,   303,   306,   309,   312,   315,   321,   328,
-     334,   338,   346,   349,   355,   359,   366,   371,   378,   386,
-     389,   392,   398,   401,   404,   407,   414,   415,   416,   417,
-     425,   426,   429,   432,   439,   440,   443,   449,   450,   454,
-     461,   462,   465,   468,   471,   477,   478,   481,   487,   488,
-     495,   496,   503,   504,   511,   512,   518,   519,   525,   526,
-     532,   533,   539,   540,   546,   547,   548,   549,   553,   554,
-     555,   559,   563,   567,   571,   578,   581,   587,   594,   601,
-     604,   607,   611,   615,   619,   623,   627,   634,   641,   644,
-     651,   659,   676,   686,   689,   695,   699,   703,   707,   714,
-     721,   724,   728,   732,   737,   744,   748,   752,   756,   761,
-     768,   772,   778,   781,   787,   791,   798,   804,   808,   812,
-     815,   818,   827,   832,   836,   839,   842,   845,   848,   852,
-     855,   859,   862,   865,   868,   871,   874,   881,   888,   891,
-     894,   900,   907,   910,   916,   919,   922,   925,   931,   934,
-     941,   946,   953,   958,   969,   972,   975,   978,   981,   984,
-     988,   992,   996,  1000,  1004,  1008,  1012,  1016,  1020,  1024,
-    1028,  1032,  1036,  1040,  1044,  1048,  1052,  1056,  1060,  1064,
-    1068,  1075,  1078,  1081,  1084,  1087,  1090,  1093,  1096,  1099,
-    1102,  1105,  1108,  1111,  1114,  1117,  1120,  1123,  1126,  1129,
-    1139,  1146,  1153,  1156,  1159,  1162,  1165,  1168,  1171,  1174,
-    1177,  1180,  1183,  1186,  1189,  1192,  1195,  1203,  1203,  1206,
-    1206,  1212,  1215,  1221,  1224,  1231,  1235,  1241,  1244,  1250,
-    1254,  1258,  1259,  1265,  1266,  1267,  1268,  1269,  1270,  1271,
-    1275,  1279,  1279,  1279,  1286,  1287,  1291,  1291,  1292,  1292,
-    1297,  1301,  1308,  1312,  1319,  1320,  1324,  1330,  1334,  1343,
-    1343,  1350,  1353,  1359,  1363,  1369,  1369,  1374,  1374,  1378,
-    1378,  1386,  1389,  1395,  1398,  1404,  1408,  1415,  1418,  1421,
-    1424,  1427,  1435,  1441,  1447,  1450,  1456,  1456
+       0,   251,   251,   252,   255,   265,   268,   273,   278,   283,
+     288,   297,   303,   306,   309,   312,   315,   318,   324,   331,
+     337,   341,   349,   352,   358,   362,   369,   374,   381,   389,
+     392,   395,   401,   404,   407,   410,   417,   418,   419,   420,
+     428,   429,   432,   435,   442,   443,   446,   452,   453,   457,
+     464,   465,   468,   471,   474,   480,   481,   484,   490,   491,
+     498,   499,   506,   507,   514,   515,   521,   522,   528,   529,
+     535,   536,   542,   543,   549,   550,   551,   552,   556,   557,
+     558,   562,   566,   570,   574,   581,   584,   590,   597,   604,
+     607,   610,   614,   618,   622,   626,   630,   637,   644,   647,
+     654,   662,   679,   689,   692,   698,   702,   706,   710,   717,
+     724,   727,   731,   735,   740,   747,   751,   755,   759,   764,
+     771,   775,   781,   784,   790,   794,   801,   807,   811,   815,
+     818,   821,   830,   835,   839,   842,   845,   848,   851,   855,
+     858,   862,   865,   868,   871,   874,   877,   884,   891,   894,
+     897,   903,   910,   913,   919,   922,   925,   928,   934,   937,
+     944,   949,   956,   961,   972,   975,   978,   981,   984,   987,
+     991,   995,   999,  1003,  1007,  1011,  1015,  1019,  1023,  1027,
+    1031,  1035,  1039,  1043,  1047,  1051,  1055,  1059,  1063,  1067,
+    1071,  1078,  1081,  1084,  1087,  1090,  1093,  1096,  1099,  1102,
+    1105,  1108,  1111,  1114,  1117,  1120,  1123,  1126,  1129,  1132,
+    1142,  1149,  1156,  1159,  1162,  1165,  1168,  1171,  1174,  1177,
+    1180,  1183,  1186,  1189,  1192,  1195,  1198,  1206,  1206,  1209,
+    1209,  1215,  1218,  1224,  1227,  1234,  1238,  1244,  1247,  1253,
+    1257,  1261,  1262,  1268,  1269,  1270,  1271,  1272,  1273,  1274,
+    1278,  1282,  1282,  1282,  1289,  1290,  1294,  1294,  1295,  1295,
+    1300,  1304,  1311,  1315,  1322,  1323,  1327,  1333,  1337,  1346,
+    1346,  1353,  1356,  1362,  1366,  1372,  1372,  1377,  1377,  1381,
+    1381,  1389,  1392,  1398,  1401,  1407,  1411,  1418,  1421,  1424,
+    1427,  1430,  1438,  1444,  1450,  1453,  1459,  1459
 };
 #endif
 
@@ -4435,7 +4437,7 @@ yyreduce:
   case 233:
 
     {
-        (yyval.interm.fieldList) = context->addStructDeclaratorList((yyvsp[-2].interm.type), (yyvsp[-1].interm.fieldList));
+        (yyval.interm.fieldList) = context->addStructDeclaratorList((yyvsp[-2].interm.type), (yyvsp[-1].interm.declaratorList));
     }
 
     break;
@@ -4444,7 +4446,7 @@ yyreduce:
 
     {
         // ES3 Only, but errors should be handled elsewhere
-        (yyval.interm.fieldList) = context->addStructDeclaratorListWithQualifiers(*(yyvsp[-3].interm.typeQualifierBuilder), &(yyvsp[-2].interm.type), (yyvsp[-1].interm.fieldList));
+        (yyval.interm.fieldList) = context->addStructDeclaratorListWithQualifiers(*(yyvsp[-3].interm.typeQualifierBuilder), &(yyvsp[-2].interm.type), (yyvsp[-1].interm.declaratorList));
     }
 
     break;
@@ -4452,8 +4454,8 @@ yyreduce:
   case 235:
 
     {
-        (yyval.interm.fieldList) = NewPoolTFieldList();
-        (yyval.interm.fieldList)->push_back((yyvsp[0].interm.field));
+        (yyval.interm.declaratorList) = new TDeclaratorList();
+        (yyval.interm.declaratorList)->push_back((yyvsp[0].interm.declarator));
     }
 
     break;
@@ -4461,7 +4463,7 @@ yyreduce:
   case 236:
 
     {
-        (yyval.interm.fieldList)->push_back((yyvsp[0].interm.field));
+        (yyval.interm.declaratorList)->push_back((yyvsp[0].interm.declarator));
     }
 
     break;
@@ -4469,7 +4471,7 @@ yyreduce:
   case 237:
 
     {
-        (yyval.interm.field) = context->parseStructDeclarator((yyvsp[0].lex).string, (yylsp[0]));
+        (yyval.interm.declarator) = context->parseStructDeclarator((yyvsp[0].lex).string, (yylsp[0]));
     }
 
     break;
@@ -4477,7 +4479,7 @@ yyreduce:
   case 238:
 
     {
-        (yyval.interm.field) = context->parseStructArrayDeclarator((yyvsp[-1].lex).string, (yylsp[-1]), *((yyvsp[0].interm.arraySizes)), (yylsp[0]));
+        (yyval.interm.declarator) = context->parseStructArrayDeclarator((yyvsp[-1].lex).string, (yylsp[-1]), (yyvsp[0].interm.arraySizes));
     }
 
     break;

src/compiler/translator/glslang_tab.h

@@ -246,7 +246,8 @@ union YYSTYPE
             TQualifier qualifier;
             TFunction *function;
             TParameter param;
-            TField *field;
+            TDeclarator *declarator;
+            TDeclaratorList *declaratorList;
             TFieldList *fieldList;
             TQualifierWrapperBase *qualifierWrapper;
             TTypeQualifierBuilder *typeQualifierBuilder;