Parse array specifier with a separate grammar rule

src/compiler/translator/ParseContext.h

@@ -148,7 +148,7 @@ class TParseContext : angle::NonCopyable
     void nonEmptyDeclarationErrorCheck(const TPublicType &publicType,
                                        const TSourceLoc &identifierLocation);
     // Done only for empty declarations.
-    void emptyDeclarationErrorCheck(const TPublicType &publicType, const TSourceLoc &location);
+    void emptyDeclarationErrorCheck(const TType &type, const TSourceLoc &location);
 
     void checkLayoutQualifierSupported(const TSourceLoc &location,
                                        const TString &layoutQualifierName,
@@ -180,7 +180,7 @@ class TParseContext : angle::NonCopyable
     // is not needed in the AST.
     bool executeInitializer(const TSourceLoc &line,
                             const TString &identifier,
-                            const TPublicType &pType,
+                            TType type,
                             TIntermTyped *initializer,
                             TIntermBinary **initNode);
     TIntermNode *addConditionInitializer(const TPublicType &pType,
@@ -205,11 +205,11 @@ class TParseContext : angle::NonCopyable
     TIntermDeclaration *parseSingleDeclaration(TPublicType &publicType,
                                                const TSourceLoc &identifierOrTypeLocation,
                                                const TString &identifier);
-    TIntermDeclaration *parseSingleArrayDeclaration(TPublicType &publicType,
+    TIntermDeclaration *parseSingleArrayDeclaration(TPublicType &elementType,
                                                     const TSourceLoc &identifierLocation,
                                                     const TString &identifier,
                                                     const TSourceLoc &indexLocation,
-                                                    TIntermTyped *indexExpression);
+                                                    unsigned int arraySize);
     TIntermDeclaration *parseSingleInitDeclaration(const TPublicType &publicType,
                                                    const TSourceLoc &identifierLocation,
                                                    const TString &identifier,
@@ -218,11 +218,11 @@ class TParseContext : angle::NonCopyable
 
     // Parse a declaration like "type a[n] = initializer"
     // Note that this does not apply to declarations like "type[n] a = initializer"
-    TIntermDeclaration *parseSingleArrayInitDeclaration(TPublicType &publicType,
+    TIntermDeclaration *parseSingleArrayInitDeclaration(TPublicType &elementType,
                                                         const TSourceLoc &identifierLocation,
                                                         const TString &identifier,
                                                         const TSourceLoc &indexLocation,
-                                                        TIntermTyped *indexExpression,
+                                                        unsigned int arraySize,
                                                         const TSourceLoc &initLocation,
                                                         TIntermTyped *initializer);
 
@@ -236,11 +236,11 @@ class TParseContext : angle::NonCopyable
                          const TSourceLoc &identifierLocation,
                          const TString &identifier,
                          TIntermDeclaration *declarationOut);
-    void parseArrayDeclarator(TPublicType &publicType,
+    void parseArrayDeclarator(TPublicType &elementType,
                               const TSourceLoc &identifierLocation,
                               const TString &identifier,
                               const TSourceLoc &arrayLocation,
-                              TIntermTyped *indexExpression,
+                              unsigned int arraySize,
                               TIntermDeclaration *declarationOut);
     void parseInitDeclarator(const TPublicType &publicType,
                              const TSourceLoc &identifierLocation,
@@ -250,11 +250,11 @@ class TParseContext : angle::NonCopyable
                              TIntermDeclaration *declarationOut);
 
     // Parse a declarator like "a[n] = initializer"
-    void parseArrayInitDeclarator(const TPublicType &publicType,
+    void parseArrayInitDeclarator(const TPublicType &elementType,
                                   const TSourceLoc &identifierLocation,
                                   const TString &identifier,
                                   const TSourceLoc &indexLocation,
-                                  TIntermTyped *indexExpression,
+                                  unsigned int arraySize,
                                   const TSourceLoc &initLocation,
                                   TIntermTyped *initializer,
                                   TIntermDeclaration *declarationOut);
@@ -281,11 +281,12 @@ class TParseContext : angle::NonCopyable
     TParameter parseParameterDeclarator(const TPublicType &publicType,
                                         const TString *name,
                                         const TSourceLoc &nameLoc);
-    TParameter parseParameterArrayDeclarator(const TString *identifier,
-                                             const TSourceLoc &identifierLoc,
-                                             TIntermTyped *arraySize,
+
+    TParameter parseParameterArrayDeclarator(const TString *name,
+                                             const TSourceLoc &nameLoc,
+                                             unsigned int arraySize,
                                              const TSourceLoc &arrayLoc,
-                                             TPublicType *type);
+                                             TPublicType *elementType);
 
     TIntermTyped *addIndexExpression(TIntermTyped *baseExpression,
                                      const TSourceLoc &location,
@@ -453,7 +454,11 @@ class TParseContext : angle::NonCopyable
 
     void checkCanBeDeclaredWithoutInitializer(const TSourceLoc &line,
                                               const TString &identifier,
-                                              TPublicType *type);
+                                              TType *type);
+
+    TParameter parseParameterDeclarator(TType *type,
+                                        const TString *name,
+                                        const TSourceLoc &nameLoc);
 
     bool checkIsValidTypeAndQualifierForArray(const TSourceLoc &indexLocation,
                                               const TPublicType &elementType);
@@ -502,6 +507,13 @@ class TParseContext : angle::NonCopyable
                                                             TType type,
                                                             const TSourceLoc &line);
 
+    // Will size any unsized array type so unsized arrays won't need to be taken into account
+    // further along the line in parsing.
+    void checkIsNotUnsizedArray(const TSourceLoc &line,
+                                const char *errorMessage,
+                                const char *token,
+                                TType *arrayType);
+
     TIntermTyped *addBinaryMathInternal(TOperator op,
                                         TIntermTyped *left,
                                         TIntermTyped *right,

src/compiler/translator/Types.cpp

@@ -577,6 +577,12 @@ void TType::sizeUnsizedArrays(const TVector<unsigned int> &arraySizes)
     invalidateMangledName();
 }
 
+void TType::sizeOutermostUnsizedArray(unsigned int arraySize)
+{
+    ASSERT(isArray());
+    mArraySizes.back() = arraySize;
+}
+
 TStructure::TStructure(TSymbolTable *symbolTable, const TString *name, TFieldList *fields)
     : TFieldListCollection(name, fields),
       mDeepestNesting(0),

src/compiler/translator/Types.h

@@ -349,6 +349,9 @@ class TType
     // than there are sizes in arraySizes, defaults to setting array sizes to 1.
     void sizeUnsizedArrays(const TVector<unsigned int> &arraySizes);
 
+    // Will size the outermost array according to arraySize.
+    void sizeOutermostUnsizedArray(unsigned int arraySize);
+
     // Note that the array element type might still be an array type in GLSL ES version >= 3.10.
     void toArrayElementType()
     {
@@ -636,7 +639,6 @@ struct TPublicType
         return typeSpecifierNonArray.userDef->containsType(t);
     }
 
-    bool isUnsizedArray() const { return array && arraySize == 0; }
     void setArraySize(int s)
     {
         array     = true;

src/compiler/translator/glslang.y

@@ -86,6 +86,7 @@ using namespace sh;
             TIntermCase *intermCase;
         };
         union {
+            unsigned int arraySize;
             TTypeSpecifierNonArray typeSpecifierNonArray;
             TPublicType type;
             TPrecision precision;
@@ -218,6 +219,8 @@ extern void yyerror(YYLTYPE* yylloc, TParseContext* context, void *scanner, cons
 %type <interm.param> parameter_declaration parameter_declarator parameter_type_specifier
 %type <interm.layoutQualifier> layout_qualifier_id_list layout_qualifier_id
 
+%type <interm.arraySize> array_specifier
+
 %type <interm.type> fully_specified_type type_specifier
 
 %type <interm.precision> precision_qualifier
@@ -681,8 +684,8 @@ parameter_declarator
     : type_specifier identifier {
         $$ = context->parseParameterDeclarator($1, $2.string, @2);
     }
-    | type_specifier identifier LEFT_BRACKET constant_expression RIGHT_BRACKET {
-        $$ = context->parseParameterArrayDeclarator($2.string, @2, $4, @3, &$1);
+    | type_specifier identifier array_specifier {
+        $$ = context->parseParameterArrayDeclarator($2.string, @2, $3, @3, &$1);
     }
     ;
 
@@ -720,19 +723,14 @@ init_declarator_list
         $$ = $1;
         context->parseDeclarator($$.type, @3, *$3.string, $$.intermDeclaration);
     }
-    | init_declarator_list COMMA identifier LEFT_BRACKET constant_expression RIGHT_BRACKET {
-        $$ = $1;
-        context->parseArrayDeclarator($$.type, @3, *$3.string, @4, $5, $$.intermDeclaration);
-    }
-    | init_declarator_list COMMA identifier LEFT_BRACKET RIGHT_BRACKET EQUAL initializer {
-        ES3_OR_NEWER("[]", @3, "implicitly sized array");
+    | init_declarator_list COMMA identifier array_specifier {
         $$ = $1;
-        context->parseArrayInitDeclarator($$.type, @3, *$3.string, @4, nullptr, @6, $7, $$.intermDeclaration);
+        context->parseArrayDeclarator($$.type, @3, *$3.string, @4, $4, $$.intermDeclaration);
     }
-    | init_declarator_list COMMA identifier LEFT_BRACKET constant_expression RIGHT_BRACKET EQUAL initializer {
-        ES3_OR_NEWER("=", @7, "first-class arrays (array initializer)");
+    | init_declarator_list COMMA identifier array_specifier EQUAL initializer {
+        ES3_OR_NEWER("=", @5, "first-class arrays (array initializer)");
         $$ = $1;
-        context->parseArrayInitDeclarator($$.type, @3, *$3.string, @4, $5, @7, $8, $$.intermDeclaration);
+        context->parseArrayInitDeclarator($$.type, @3, *$3.string, @4, $4, @5, $6, $$.intermDeclaration);
     }
     | init_declarator_list COMMA identifier EQUAL initializer {
         $$ = $1;
@@ -749,24 +747,14 @@ single_declaration
         $$.type = $1;
         $$.intermDeclaration = context->parseSingleDeclaration($$.type, @2, *$2.string);
     }
-    | fully_specified_type identifier LEFT_BRACKET RIGHT_BRACKET {
-        ES3_1_ONLY("[]", @3, "implicitly sized array declaration");
+    | fully_specified_type identifier array_specifier {
         $$.type = $1;
-        $$.intermDeclaration = context->parseSingleArrayDeclaration($$.type, @2, *$2.string, @3, nullptr);
+        $$.intermDeclaration = context->parseSingleArrayDeclaration($$.type, @2, *$2.string, @3, $3);
     }
-    | fully_specified_type identifier LEFT_BRACKET constant_expression RIGHT_BRACKET {
+    | fully_specified_type identifier array_specifier EQUAL initializer {
+        ES3_OR_NEWER("[]", @3, "first-class arrays (array initializer)");
         $$.type = $1;
-        $$.intermDeclaration = context->parseSingleArrayDeclaration($$.type, @2, *$2.string, @3, $4);
-    }
-    | fully_specified_type identifier LEFT_BRACKET RIGHT_BRACKET EQUAL initializer {
-        ES3_OR_NEWER("[]", @3, "implicitly sized array");
-        $$.type = $1;
-        $$.intermDeclaration = context->parseSingleArrayInitDeclaration($$.type, @2, *$2.string, @3, nullptr, @5, $6);
-    }
-    | fully_specified_type identifier LEFT_BRACKET constant_expression RIGHT_BRACKET EQUAL initializer {
-        ES3_OR_NEWER("=", @6, "first-class arrays (array initializer)");
-        $$.type = $1;
-        $$.intermDeclaration = context->parseSingleArrayInitDeclaration($$.type, @2, *$2.string, @3, $4, @6, $7);
+        $$.intermDeclaration = context->parseSingleArrayInitDeclaration($$.type, @2, *$2.string, @3, $3, @4, $5);
     }
     | fully_specified_type identifier EQUAL initializer {
         $$.type = $1;
@@ -941,15 +929,22 @@ type_specifier_no_prec
     : type_specifier_nonarray {
         $$.initialize($1, (context->symbolTable.atGlobalLevel() ? EvqGlobal : EvqTemporary));
     }
-    | type_specifier_nonarray LEFT_BRACKET RIGHT_BRACKET {
-        ES3_OR_NEWER("[]", @2, "implicitly sized array");
+    | type_specifier_nonarray array_specifier {
         $$.initialize($1, (context->symbolTable.atGlobalLevel() ? EvqGlobal : EvqTemporary));
-        $$.setArraySize(0);
+        $$.setArraySize($2);
     }
-    | type_specifier_nonarray LEFT_BRACKET constant_expression RIGHT_BRACKET {
-        $$.initialize($1, (context->symbolTable.atGlobalLevel() ? EvqGlobal : EvqTemporary));
-        unsigned int size = context->checkIsValidArraySize(@2, $3);
-        $$.setArraySize(size);
+    ;
+
+array_specifier
+    : LEFT_BRACKET RIGHT_BRACKET {
+        ES3_OR_NEWER("[]", @1, "implicitly sized array");
+        $$ = 0u;
+    }
+    | LEFT_BRACKET constant_expression RIGHT_BRACKET {
+        unsigned int size = context->checkIsValidArraySize(@1, $2);
+        // Make the type an array even if size check failed.
+        // This ensures useless error messages regarding a variable's non-arrayness won't follow.
+        $$ = size;
     }
     ;
 

src/compiler/translator/glslang_tab.h

@@ -238,6 +238,7 @@ union YYSTYPE
             TIntermCase *intermCase;
         };
         union {
+            unsigned int arraySize;
             TTypeSpecifierNonArray typeSpecifierNonArray;
             TPublicType type;
             TPrecision precision;

src/tests/compiler_tests/ShaderValidation_test.cpp

@@ -5104,3 +5104,51 @@ TEST_F(FragmentShaderValidationTest, UnsizedInputs)
         FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
     }
 }
+
+// Test that unsized struct members are not allowed.
+TEST_F(FragmentShaderValidationTest, UnsizedStructMember)
+{
+    const std::string &shaderString =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 color;
+
+        struct S
+        {
+            int[] foo;
+        };
+
+        void main()
+        {
+            color = vec4(1.0);
+        })";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
+    }
+}
+
+// Test that unsized parameters without a name are not allowed.
+// GLSL ES 3.10 section 6.1 Function Definitions.
+TEST_F(FragmentShaderValidationTest, UnsizedNamelessParameter)
+{
+    const std::string &shaderString =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 color;
+
+        void foo(int[]);
+
+        void main()
+        {
+            color = vec4(1.0);
+        })";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
+    }
+}