Make checking for array non-constness depend on input version

First, remove duplicate check for const qualifier on arrays. Only keep
the check inside arrayQualifierErrorCheck().

Second, ESSL3 will introduce array initializers and by extension constant
arrays, so it should allow const qualifier on arrays. These checks are
somewhat superfluous in ESSL1 as well, since the parser already checks
for missing initializers, but it's useful to keep the informative error
messages around.

Add a few tests to make sure that when the ESSL3 implementation
progresses, it still checks for missing initializers on constant arrays.

TEST=angle_unittests
BUG=angleproject:941

Change-Id: Id871c872c5b92e2a5bf81c00080ac23004916a75
Reviewed-on: https://chromium-review.googlesource.com/264671
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>

src/compiler/translator/ParseContext.cpp

@@ -750,9 +750,11 @@ bool TParseContext::arraySizeErrorCheck(const TSourceLoc& line, TIntermTyped* ex
 //
 // Returns true if there is an error.
 //
-bool TParseContext::arrayQualifierErrorCheck(const TSourceLoc& line, TPublicType type)
+bool TParseContext::arrayQualifierErrorCheck(const TSourceLoc &line, const TPublicType &type)
 {
-    if ((type.qualifier == EvqAttribute) || (type.qualifier == EvqVertexIn) || (type.qualifier == EvqConst)) {
+    if ((type.qualifier == EvqAttribute) || (type.qualifier == EvqVertexIn) ||
+        (type.qualifier == EvqConst && shaderVersion < 300))
+    {
         error(line, "cannot declare arrays of this qualifier", TType(type).getCompleteString().c_str());
         return true;
     }
@@ -861,18 +863,17 @@ bool TParseContext::arrayErrorCheck(const TSourceLoc& line, const TString& ident
 //
 // Returns true if there was an error.
 //
-bool TParseContext::nonInitConstErrorCheck(const TSourceLoc& line, const TString& identifier, TPublicType& type, bool array)
+bool TParseContext::nonInitConstErrorCheck(const TSourceLoc &line, const TString &identifier, TPublicType *type)
 {
-    if (type.qualifier == EvqConst)
+    ASSERT(type != nullptr);
+    if (type->qualifier == EvqConst)
     {
         // Make the qualifier make sense.
-        type.qualifier = EvqTemporary;
-        
-        if (array)
-        {
-            error(line, "arrays may not be declared constant since they cannot be initialized", identifier.c_str());
-        }
-        else if (type.isStructureContainingArrays())
+        type->qualifier = EvqTemporary;
+
+        // Generate informative error messages for ESSL1.
+        // In ESSL3 arrays and structures containing arrays can be constant.
+        if (shaderVersion < 300 && type->isStructureContainingArrays())
         {
             error(line, "structures containing arrays may not be declared constant since they cannot be initialized", identifier.c_str());
         }
@@ -883,7 +884,6 @@ bool TParseContext::nonInitConstErrorCheck(const TSourceLoc& line, const TString
 
         return true;
     }
-
     return false;
 }
 
@@ -1306,8 +1306,7 @@ TIntermAggregate* TParseContext::parseSingleDeclaration(TPublicType &publicType,
         if (singleDeclarationErrorCheck(publicType, identifierLocation, identifier))
             recover();
 
-        // this error check can mutate the type
-        if (nonInitConstErrorCheck(identifierLocation, identifier, publicType, false))
+        if (nonInitConstErrorCheck(identifierLocation, identifier, &publicType))
             recover();
 
         TVariable* variable = 0;
@@ -1329,8 +1328,7 @@ TIntermAggregate* TParseContext::parseSingleArrayDeclaration(TPublicType &public
     if (singleDeclarationErrorCheck(publicType, identifierLocation, identifier))
         recover();
 
-    // this error check can mutate the type
-    if (nonInitConstErrorCheck(identifierLocation, identifier, publicType, true))
+    if (nonInitConstErrorCheck(identifierLocation, identifier, &publicType))
         recover();
 
     if (arrayTypeErrorCheck(indexLocation, publicType) || arrayQualifierErrorCheck(indexLocation, publicType))
@@ -1435,7 +1433,7 @@ TIntermAggregate* TParseContext::parseDeclarator(TPublicType &publicType, TInter
     if (locationDeclaratorListCheck(identifierLocation, publicType))
         recover();
 
-    if (nonInitConstErrorCheck(identifierLocation, identifier, publicType, false))
+    if (nonInitConstErrorCheck(identifierLocation, identifier, &publicType))
         recover();
 
     TVariable* variable = 0;
@@ -1455,7 +1453,7 @@ TIntermAggregate* TParseContext::parseArrayDeclarator(TPublicType &publicType, c
     if (locationDeclaratorListCheck(identifierLocation, publicType))
         recover();
 
-    if (nonInitConstErrorCheck(identifierLocation, identifier, publicType, true))
+    if (nonInitConstErrorCheck(identifierLocation, identifier, &publicType))
         recover();
 
     if (arrayTypeErrorCheck(arrayLocation, publicType) || arrayQualifierErrorCheck(arrayLocation, publicType))

src/compiler/translator/ParseContext.h

@@ -95,7 +95,7 @@ struct TParseContext {
     bool globalErrorCheck(const TSourceLoc& line, bool global, const char* token);
     bool constructorErrorCheck(const TSourceLoc& line, TIntermNode*, TFunction&, TOperator, TType*);
     bool arraySizeErrorCheck(const TSourceLoc& line, TIntermTyped* expr, int& size);
-    bool arrayQualifierErrorCheck(const TSourceLoc& line, TPublicType type);
+    bool arrayQualifierErrorCheck(const TSourceLoc &line, const TPublicType &type);
     bool arrayTypeErrorCheck(const TSourceLoc& line, TPublicType type);
     bool arrayErrorCheck(const TSourceLoc& line, const TString& identifier, const TPublicType &type, TVariable*& variable);
     bool voidErrorCheck(const TSourceLoc&, const TString&, const TPublicType&);
@@ -105,7 +105,7 @@ struct TParseContext {
     bool structQualifierErrorCheck(const TSourceLoc& line, const TPublicType& pType);
     bool locationDeclaratorListCheck(const TSourceLoc& line, const TPublicType &pType);
     bool parameterSamplerErrorCheck(const TSourceLoc& line, TQualifier qualifier, const TType& type);
-    bool nonInitConstErrorCheck(const TSourceLoc& line, const TString& identifier, TPublicType& type, bool array);
+    bool nonInitConstErrorCheck(const TSourceLoc &line, const TString &identifier, TPublicType *type);
     bool nonInitErrorCheck(const TSourceLoc& line, const TString& identifier, const TPublicType& type, TVariable*& variable);
     bool paramErrorCheck(const TSourceLoc& line, TQualifier qualifier, TQualifier paramQualifier, TType* type);
     bool extensionErrorCheck(const TSourceLoc& line, const TString&);

src/tests/compiler_tests/MalformedShader_test.cpp

@@ -213,3 +213,67 @@ TEST_F(MalformedShaderTest, ArrayWithNoSizeInInitializerList)
         FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
     }
 }
+
+// Const variables need an initializer.
+TEST_F(MalformedShaderTest, ConstVarNotInitialized)
+{
+    const std::string &shaderString =
+        "#version 300 es;"
+        "precision mediump float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "   const float a;"
+        "   my_FragColor = vec4(1.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Const variables need an initializer. In ESSL1 const structs containing
+// arrays are not allowed at all since it's impossible to initialize them.
+// Even though this test is for ESSL3 the only thing that's critical for
+// ESSL1 is the non-initialization check that's used for both language versions.
+// Whether ESSL1 compilation generates the most helpful error messages is a
+// secondary concern.
+TEST_F(MalformedShaderTest, ConstStructNotInitialized)
+{
+    const std::string &shaderString =
+        "#version 300 es;"
+        "precision mediump float;\n"
+        "struct S {\n"
+        "   float a[3];\n"
+        "};\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "   const S b;"
+        "   my_FragColor = vec4(1.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}
+
+// Const variables need an initializer. In ESSL1 const arrays are not allowed
+// at all since it's impossible to initialize them.
+// Even though this test is for ESSL3 the only thing that's critical for
+// ESSL1 is the non-initialization check that's used for both language versions.
+// Whether ESSL1 compilation generates the most helpful error messages is a
+// secondary concern.
+TEST_F(MalformedShaderTest, ConstArrayNotInitialized)
+{
+    const std::string &shaderString =
+        "#version 300 es;"
+        "precision mediump float;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "   const float a[3];"
+        "   my_FragColor = vec4(1.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+}