Fix switch/case last case validation for ESSL 3.10
No statement should be required after the last case label of a switch
statement in ESSL 3.10. The validation is still kept for ESSL 3.00 for
dEQP compatibility. If the dEQP tests are changed in the future, we
might consider just issuing a warning regardless of shader version.
BUG=angleproject:2189
TEST=angle_unittests
Change-Id: Ic53e71e0176668a7dbffa315712885846e217f03
Reviewed-on: https://chromium-review.googlesource.com/727802
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 e5f9f59..6631e49 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -4640,8 +4640,9 @@
}
ASSERT(statementList);
- if (!ValidateSwitchStatementList(switchType, mDiagnostics, statementList, loc))
+ if (!ValidateSwitchStatementList(switchType, mShaderVersion, mDiagnostics, statementList, loc))
{
+ ASSERT(mDiagnostics->numErrors() > 0);
return nullptr;
}
diff --git a/src/compiler/translator/ValidateSwitch.cpp b/src/compiler/translator/ValidateSwitch.cpp
index bc9ba34..9f7a264 100644
--- a/src/compiler/translator/ValidateSwitch.cpp
+++ b/src/compiler/translator/ValidateSwitch.cpp
@@ -19,6 +19,7 @@
{
public:
static bool validate(TBasicType switchType,
+ int shaderVersion,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc);
@@ -39,11 +40,12 @@
bool visitBranch(Visit, TIntermBranch *) override;
private:
- ValidateSwitch(TBasicType switchType, TDiagnostics *context);
+ ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *context);
bool validateInternal(const TSourceLoc &loc);
TBasicType mSwitchType;
+ int mShaderVersion;
TDiagnostics *mDiagnostics;
bool mCaseTypeMismatch;
bool mFirstCaseFound;
@@ -58,19 +60,21 @@
};
bool ValidateSwitch::validate(TBasicType switchType,
+ int shaderVersion,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc)
{
- ValidateSwitch validate(switchType, diagnostics);
+ ValidateSwitch validate(switchType, shaderVersion, diagnostics);
ASSERT(statementList);
statementList->traverse(&validate);
return validate.validateInternal(loc);
}
-ValidateSwitch::ValidateSwitch(TBasicType switchType, TDiagnostics *diagnostics)
+ValidateSwitch::ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *diagnostics)
: TIntermTraverser(true, false, true),
mSwitchType(switchType),
+ mShaderVersion(shaderVersion),
mDiagnostics(diagnostics),
mCaseTypeMismatch(false),
mFirstCaseFound(false),
@@ -277,24 +281,39 @@
{
mDiagnostics->error(loc, "statement before the first label", "switch");
}
+ bool lastStatementWasCaseError = false;
if (mLastStatementWasCase)
{
- mDiagnostics->error(
- loc, "no statement between the last label and the end of the switch statement",
- "switch");
+ 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");
+ }
}
- return !mStatementBeforeCase && !mLastStatementWasCase && !mCaseInsideControlFlow &&
+ return !mStatementBeforeCase && !lastStatementWasCaseError && !mCaseInsideControlFlow &&
!mCaseTypeMismatch && mDefaultCount <= 1 && !mDuplicateCases;
}
} // anonymous namespace
bool ValidateSwitchStatementList(TBasicType switchType,
+ int shaderVersion,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc)
{
- return ValidateSwitch::validate(switchType, diagnostics, statementList, loc);
+ return ValidateSwitch::validate(switchType, shaderVersion, diagnostics, statementList, loc);
}
} // namespace sh
diff --git a/src/compiler/translator/ValidateSwitch.h b/src/compiler/translator/ValidateSwitch.h
index 757820a..2d2dd70 100644
--- a/src/compiler/translator/ValidateSwitch.h
+++ b/src/compiler/translator/ValidateSwitch.h
@@ -18,6 +18,7 @@
// 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);
diff --git a/src/tests/compiler_tests/ShaderValidation_test.cpp b/src/tests/compiler_tests/ShaderValidation_test.cpp
index 0f91fe6..617d76f 100644
--- a/src/tests/compiler_tests/ShaderValidation_test.cpp
+++ b/src/tests/compiler_tests/ShaderValidation_test.cpp
@@ -5063,3 +5063,27 @@
FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog;
}
}
+
+// Test that nothing is needed after the final case in a switch statement in ESSL 3.10.
+TEST_F(FragmentShaderValidationTest, SwitchFinalCaseEmptyESSL310)
+{
+ const std::string &shaderString =
+ R"(#version 310 es
+
+ precision mediump float;
+ uniform int i;
+ void main()
+ {
+ switch (i)
+ {
+ case 0:
+ break;
+ default:
+ }
+ })";
+
+ if (!compile(shaderString))
+ {
+ FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog;
+ }
+}
