Last case in switch statement can't be empty in ESSL 3.10

src/compiler/translator/ParseContext.cpp

@@ -4840,7 +4840,7 @@ TIntermSwitch *TParseContext::addSwitch(TIntermTyped *init,
     }
 
     ASSERT(statementList);
-    if (!ValidateSwitchStatementList(switchType, mShaderVersion, mDiagnostics, statementList, loc))
+    if (!ValidateSwitchStatementList(switchType, mDiagnostics, statementList, loc))
     {
         ASSERT(mDiagnostics->numErrors() > 0);
         return nullptr;

src/compiler/translator/ValidateSwitch.cpp

@@ -19,7 +19,6 @@ class ValidateSwitch : public TIntermTraverser
 {
   public:
     static bool validate(TBasicType switchType,
-                         int shaderVersion,
                          TDiagnostics *diagnostics,
                          TIntermBlock *statementList,
                          const TSourceLoc &loc);
@@ -40,12 +39,11 @@ class ValidateSwitch : public TIntermTraverser
     bool visitBranch(Visit, TIntermBranch *) override;
 
   private:
-    ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *context);
+    ValidateSwitch(TBasicType switchType, TDiagnostics *context);
 
     bool validateInternal(const TSourceLoc &loc);
 
     TBasicType mSwitchType;
-    int mShaderVersion;
     TDiagnostics *mDiagnostics;
     bool mCaseTypeMismatch;
     bool mFirstCaseFound;
@@ -60,21 +58,19 @@ class ValidateSwitch : public TIntermTraverser
 };
 
 bool ValidateSwitch::validate(TBasicType switchType,
-                              int shaderVersion,
                               TDiagnostics *diagnostics,
                               TIntermBlock *statementList,
                               const TSourceLoc &loc)
 {
-    ValidateSwitch validate(switchType, shaderVersion, diagnostics);
+    ValidateSwitch validate(switchType, diagnostics);
     ASSERT(statementList);
     statementList->traverse(&validate);
     return validate.validateInternal(loc);
 }
 
-ValidateSwitch::ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *diagnostics)
+ValidateSwitch::ValidateSwitch(TBasicType switchType, TDiagnostics *diagnostics)
     : TIntermTraverser(true, false, true),
       mSwitchType(switchType),
-      mShaderVersion(shaderVersion),
       mDiagnostics(diagnostics),
       mCaseTypeMismatch(false),
       mFirstCaseFound(false),
@@ -281,39 +277,27 @@ bool ValidateSwitch::validateInternal(const TSourceLoc &loc)
     {
         mDiagnostics->error(loc, "statement before the first label", "switch");
     }
-    bool lastStatementWasCaseError = false;
     if (mLastStatementWasCase)
     {
-        if (mShaderVersion == 300)
-        {
-            lastStatementWasCaseError = true;
-            // This error has been proposed to be made optional in GLSL ES 3.00, but dEQP tests
-            // still require it.
-            mDiagnostics->error(
-                loc, "no statement between the last label and the end of the switch statement",
-                "switch");
-        }
-        else
-        {
-            // The error has been removed from GLSL ES 3.10.
-            mDiagnostics->warning(
-                loc, "no statement between the last label and the end of the switch statement",
-                "switch");
-        }
+        // There have been some differences between versions of GLSL ES specs on whether this should
+        // be an error or not, but as of early 2018 the latest discussion is that this is an error
+        // also on GLSL ES versions newer than 3.00.
+        mDiagnostics->error(
+            loc, "no statement between the last label and the end of the switch statement",
+            "switch");
     }
-    return !mStatementBeforeCase && !lastStatementWasCaseError && !mCaseInsideControlFlow &&
+    return !mStatementBeforeCase && !mLastStatementWasCase && !mCaseInsideControlFlow &&
            !mCaseTypeMismatch && mDefaultCount <= 1 && !mDuplicateCases;
 }
 
 }  // anonymous namespace
 
 bool ValidateSwitchStatementList(TBasicType switchType,
-                                 int shaderVersion,
                                  TDiagnostics *diagnostics,
                                  TIntermBlock *statementList,
                                  const TSourceLoc &loc)
 {
-    return ValidateSwitch::validate(switchType, shaderVersion, diagnostics, statementList, loc);
+    return ValidateSwitch::validate(switchType, diagnostics, statementList, loc);
 }
 
 }  // namespace sh

src/compiler/translator/ValidateSwitch.h

@@ -18,7 +18,6 @@ class TIntermBlock;
 // Check for errors and output error messages on the context.
 // Returns true if there are no errors.
 bool ValidateSwitchStatementList(TBasicType switchType,
-                                 int shaderVersion,
                                  TDiagnostics *diagnostics,
                                  TIntermBlock *statementList,
                                  const TSourceLoc &loc);

src/tests/compiler_tests/ShaderValidation_test.cpp

@@ -5069,7 +5069,8 @@ TEST_F(FragmentShaderValidationTest, SwitchFinalCaseHasEmptyDeclaration)
     }
 }
 
-// Test that nothing is needed after the final case in a switch statement in ESSL 3.10.
+// The final case in a switch statement can't be empty in ESSL 3.10 either. This is the intent of
+// the spec though public spec in early 2018 didn't reflect this yet.
 TEST_F(FragmentShaderValidationTest, SwitchFinalCaseEmptyESSL310)
 {
     const std::string &shaderString =
@@ -5087,9 +5088,9 @@ TEST_F(FragmentShaderValidationTest, SwitchFinalCaseEmptyESSL310)
             }
         })";
 
-    if (!compile(shaderString))
+    if (compile(shaderString))
     {
-        FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog;
+        FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
     }
 }