Commit cbcb96fc by Olli Etuaho Committed by Commit Bot

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/727802Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 6dee4d8c
...@@ -4640,8 +4640,9 @@ TIntermSwitch *TParseContext::addSwitch(TIntermTyped *init, ...@@ -4640,8 +4640,9 @@ TIntermSwitch *TParseContext::addSwitch(TIntermTyped *init,
} }
ASSERT(statementList); ASSERT(statementList);
if (!ValidateSwitchStatementList(switchType, mDiagnostics, statementList, loc)) if (!ValidateSwitchStatementList(switchType, mShaderVersion, mDiagnostics, statementList, loc))
{ {
ASSERT(mDiagnostics->numErrors() > 0);
return nullptr; return nullptr;
} }
......
...@@ -19,6 +19,7 @@ class ValidateSwitch : public TIntermTraverser ...@@ -19,6 +19,7 @@ class ValidateSwitch : public TIntermTraverser
{ {
public: public:
static bool validate(TBasicType switchType, static bool validate(TBasicType switchType,
int shaderVersion,
TDiagnostics *diagnostics, TDiagnostics *diagnostics,
TIntermBlock *statementList, TIntermBlock *statementList,
const TSourceLoc &loc); const TSourceLoc &loc);
...@@ -39,11 +40,12 @@ class ValidateSwitch : public TIntermTraverser ...@@ -39,11 +40,12 @@ class ValidateSwitch : public TIntermTraverser
bool visitBranch(Visit, TIntermBranch *) override; bool visitBranch(Visit, TIntermBranch *) override;
private: private:
ValidateSwitch(TBasicType switchType, TDiagnostics *context); ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *context);
bool validateInternal(const TSourceLoc &loc); bool validateInternal(const TSourceLoc &loc);
TBasicType mSwitchType; TBasicType mSwitchType;
int mShaderVersion;
TDiagnostics *mDiagnostics; TDiagnostics *mDiagnostics;
bool mCaseTypeMismatch; bool mCaseTypeMismatch;
bool mFirstCaseFound; bool mFirstCaseFound;
...@@ -58,19 +60,21 @@ class ValidateSwitch : public TIntermTraverser ...@@ -58,19 +60,21 @@ class ValidateSwitch : public TIntermTraverser
}; };
bool ValidateSwitch::validate(TBasicType switchType, bool ValidateSwitch::validate(TBasicType switchType,
int shaderVersion,
TDiagnostics *diagnostics, TDiagnostics *diagnostics,
TIntermBlock *statementList, TIntermBlock *statementList,
const TSourceLoc &loc) const TSourceLoc &loc)
{ {
ValidateSwitch validate(switchType, diagnostics); ValidateSwitch validate(switchType, shaderVersion, diagnostics);
ASSERT(statementList); ASSERT(statementList);
statementList->traverse(&validate); statementList->traverse(&validate);
return validate.validateInternal(loc); return validate.validateInternal(loc);
} }
ValidateSwitch::ValidateSwitch(TBasicType switchType, TDiagnostics *diagnostics) ValidateSwitch::ValidateSwitch(TBasicType switchType, int shaderVersion, TDiagnostics *diagnostics)
: TIntermTraverser(true, false, true), : TIntermTraverser(true, false, true),
mSwitchType(switchType), mSwitchType(switchType),
mShaderVersion(shaderVersion),
mDiagnostics(diagnostics), mDiagnostics(diagnostics),
mCaseTypeMismatch(false), mCaseTypeMismatch(false),
mFirstCaseFound(false), mFirstCaseFound(false),
...@@ -277,24 +281,39 @@ bool ValidateSwitch::validateInternal(const TSourceLoc &loc) ...@@ -277,24 +281,39 @@ bool ValidateSwitch::validateInternal(const TSourceLoc &loc)
{ {
mDiagnostics->error(loc, "statement before the first label", "switch"); mDiagnostics->error(loc, "statement before the first label", "switch");
} }
bool lastStatementWasCaseError = false;
if (mLastStatementWasCase) if (mLastStatementWasCase)
{ {
mDiagnostics->error( if (mShaderVersion == 300)
loc, "no statement between the last label and the end of the switch statement", {
"switch"); 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; !mCaseTypeMismatch && mDefaultCount <= 1 && !mDuplicateCases;
} }
} // anonymous namespace } // anonymous namespace
bool ValidateSwitchStatementList(TBasicType switchType, bool ValidateSwitchStatementList(TBasicType switchType,
int shaderVersion,
TDiagnostics *diagnostics, TDiagnostics *diagnostics,
TIntermBlock *statementList, TIntermBlock *statementList,
const TSourceLoc &loc) const TSourceLoc &loc)
{ {
return ValidateSwitch::validate(switchType, diagnostics, statementList, loc); return ValidateSwitch::validate(switchType, shaderVersion, diagnostics, statementList, loc);
} }
} // namespace sh } // namespace sh
...@@ -18,6 +18,7 @@ class TIntermBlock; ...@@ -18,6 +18,7 @@ class TIntermBlock;
// Check for errors and output error messages on the context. // Check for errors and output error messages on the context.
// Returns true if there are no errors. // Returns true if there are no errors.
bool ValidateSwitchStatementList(TBasicType switchType, bool ValidateSwitchStatementList(TBasicType switchType,
int shaderVersion,
TDiagnostics *diagnostics, TDiagnostics *diagnostics,
TIntermBlock *statementList, TIntermBlock *statementList,
const TSourceLoc &loc); const TSourceLoc &loc);
......
...@@ -5063,3 +5063,27 @@ TEST_F(FragmentShaderValidationTest, SwitchFinalCaseHasEmptyDeclaration) ...@@ -5063,3 +5063,27 @@ TEST_F(FragmentShaderValidationTest, SwitchFinalCaseHasEmptyDeclaration)
FAIL() << "Shader compilation failed, expecting success:\n" << mInfoLog; 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;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment