Commit 7351c2a5 by Olli Etuaho Committed by Commit Bot

Clean up GLSL switch statement validation

Encapsulate all of the implementation inside the .cpp file, and pass just the diagnostics object instead of the whole ParseContext to the validation function. BUG=angleproject:1670 TEST=angle_unittests Change-Id: I89713b63e554dbedaa12b2270208f1fac496c54e Reviewed-on: https://chromium-review.googlesource.com/420788Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 439a55e5
......@@ -63,6 +63,11 @@ void TDiagnostics::error(const TSourceLoc &loc,
writeInfo(pp::Diagnostics::PP_ERROR, srcLoc, reason, token, extraInfo);
}
void TDiagnostics::error(const TSourceLoc &loc, const char *reason, const char *token)
{
error(loc, reason, token, "");
}
void TDiagnostics::warning(const TSourceLoc &loc,
const char *reason,
const char *token,
......
......@@ -34,6 +34,7 @@ class TDiagnostics : public pp::Diagnostics, angle::NonCopyable
const std::string &extra);
void error(const TSourceLoc &loc, const char *reason, const char *token, const char *extraInfo);
void error(const TSourceLoc &loc, const char *reason, const char *token);
void warning(const TSourceLoc &loc,
const char *reason,
const char *token,
......
......@@ -3542,7 +3542,7 @@ TIntermSwitch *TParseContext::addSwitch(TIntermTyped *init,
if (statementList)
{
if (!ValidateSwitch::validate(switchType, this, statementList, loc))
if (!ValidateSwitchStatementList(switchType, &mDiagnostics, statementList, loc))
{
return nullptr;
}
......
......@@ -6,26 +6,69 @@
#include "compiler/translator/ValidateSwitch.h"
#include "compiler/translator/ParseContext.h"
#include "compiler/translator/IntermNode.h"
#include "compiler/translator/Diagnostics.h"
namespace sh
{
namespace
{
class ValidateSwitch : public TIntermTraverser
{
public:
static bool validate(TBasicType switchType,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc);
void visitSymbol(TIntermSymbol *) override;
void visitConstantUnion(TIntermConstantUnion *) override;
bool visitBinary(Visit, TIntermBinary *) override;
bool visitUnary(Visit, TIntermUnary *) override;
bool visitTernary(Visit, TIntermTernary *) override;
bool visitIfElse(Visit visit, TIntermIfElse *) override;
bool visitSwitch(Visit, TIntermSwitch *) override;
bool visitCase(Visit, TIntermCase *node) override;
bool visitAggregate(Visit, TIntermAggregate *) override;
bool visitLoop(Visit visit, TIntermLoop *) override;
bool visitBranch(Visit, TIntermBranch *) override;
private:
ValidateSwitch(TBasicType switchType, TDiagnostics *context);
bool validateInternal(const TSourceLoc &loc);
TBasicType mSwitchType;
TDiagnostics *mDiagnostics;
bool mCaseTypeMismatch;
bool mFirstCaseFound;
bool mStatementBeforeCase;
bool mLastStatementWasCase;
int mControlFlowDepth;
bool mCaseInsideControlFlow;
int mDefaultCount;
std::set<int> mCasesSigned;
std::set<unsigned int> mCasesUnsigned;
bool mDuplicateCases;
};
bool ValidateSwitch::validate(TBasicType switchType,
TParseContext *context,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc)
{
ValidateSwitch validate(switchType, context);
ValidateSwitch validate(switchType, diagnostics);
ASSERT(statementList);
statementList->traverse(&validate);
return validate.validateInternal(loc);
}
ValidateSwitch::ValidateSwitch(TBasicType switchType, TParseContext *context)
ValidateSwitch::ValidateSwitch(TBasicType switchType, TDiagnostics *diagnostics)
: TIntermTraverser(true, false, true),
mSwitchType(switchType),
mContext(context),
mDiagnostics(diagnostics),
mCaseTypeMismatch(false),
mFirstCaseFound(false),
mStatementBeforeCase(false),
......@@ -103,7 +146,7 @@ bool ValidateSwitch::visitCase(Visit, TIntermCase *node)
const char *nodeStr = node->hasCondition() ? "case" : "default";
if (mControlFlowDepth > 0)
{
mContext->error(node->getLine(), "label statement nested inside control flow", nodeStr);
mDiagnostics->error(node->getLine(), "label statement nested inside control flow", nodeStr);
mCaseInsideControlFlow = true;
}
mFirstCaseFound = true;
......@@ -113,7 +156,7 @@ bool ValidateSwitch::visitCase(Visit, TIntermCase *node)
++mDefaultCount;
if (mDefaultCount > 1)
{
mContext->error(node->getLine(), "duplicate default label", nodeStr);
mDiagnostics->error(node->getLine(), "duplicate default label", nodeStr);
}
}
else
......@@ -127,8 +170,9 @@ bool ValidateSwitch::visitCase(Visit, TIntermCase *node)
TBasicType conditionType = condition->getBasicType();
if (conditionType != mSwitchType)
{
mContext->error(condition->getLine(),
"case label type does not match switch init-expression type", nodeStr);
mDiagnostics->error(condition->getLine(),
"case label type does not match switch init-expression type",
nodeStr);
mCaseTypeMismatch = true;
}
......@@ -137,7 +181,7 @@ bool ValidateSwitch::visitCase(Visit, TIntermCase *node)
int iConst = condition->getIConst(0);
if (mCasesSigned.find(iConst) != mCasesSigned.end())
{
mContext->error(condition->getLine(), "duplicate case label", nodeStr);
mDiagnostics->error(condition->getLine(), "duplicate case label", nodeStr);
mDuplicateCases = true;
}
else
......@@ -150,7 +194,7 @@ bool ValidateSwitch::visitCase(Visit, TIntermCase *node)
unsigned int uConst = condition->getUConst(0);
if (mCasesUnsigned.find(uConst) != mCasesUnsigned.end())
{
mContext->error(condition->getLine(), "duplicate case label", nodeStr);
mDiagnostics->error(condition->getLine(), "duplicate case label", nodeStr);
mDuplicateCases = true;
}
else
......@@ -201,16 +245,26 @@ bool ValidateSwitch::validateInternal(const TSourceLoc &loc)
{
if (mStatementBeforeCase)
{
mContext->error(loc, "statement before the first label", "switch");
mDiagnostics->error(loc, "statement before the first label", "switch");
}
if (mLastStatementWasCase)
{
mContext->error(loc,
"no statement between the last label and the end of the switch statement",
"switch");
mDiagnostics->error(
loc, "no statement between the last label and the end of the switch statement",
"switch");
}
return !mStatementBeforeCase && !mLastStatementWasCase && !mCaseInsideControlFlow &&
!mCaseTypeMismatch && mDefaultCount <= 1 && !mDuplicateCases;
}
} // anonymous namespace
bool ValidateSwitchStatementList(TBasicType switchType,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc)
{
return ValidateSwitch::validate(switchType, diagnostics, statementList, loc);
}
} // namespace sh
......@@ -7,52 +7,20 @@
#ifndef COMPILER_TRANSLATOR_VALIDATESWITCH_H_
#define COMPILER_TRANSLATOR_VALIDATESWITCH_H_
#include "compiler/translator/IntermNode.h"
#include "compiler/translator/BaseTypes.h"
#include "compiler/translator/Common.h"
namespace sh
{
class TParseContext;
class ValidateSwitch : public TIntermTraverser
{
public:
// Check for errors and output messages any remaining errors on the context.
// Returns true if there are no errors.
static bool validate(TBasicType switchType,
TParseContext *context,
TIntermBlock *statementList,
const TSourceLoc &loc);
void visitSymbol(TIntermSymbol *) override;
void visitConstantUnion(TIntermConstantUnion *) override;
bool visitBinary(Visit, TIntermBinary *) override;
bool visitUnary(Visit, TIntermUnary *) override;
bool visitTernary(Visit, TIntermTernary *) override;
bool visitIfElse(Visit visit, TIntermIfElse *) override;
bool visitSwitch(Visit, TIntermSwitch *) override;
bool visitCase(Visit, TIntermCase *node) override;
bool visitAggregate(Visit, TIntermAggregate *) override;
bool visitLoop(Visit visit, TIntermLoop *) override;
bool visitBranch(Visit, TIntermBranch *) override;
private:
ValidateSwitch(TBasicType switchType, TParseContext *context);
bool validateInternal(const TSourceLoc &loc);
TBasicType mSwitchType;
TParseContext *mContext;
bool mCaseTypeMismatch;
bool mFirstCaseFound;
bool mStatementBeforeCase;
bool mLastStatementWasCase;
int mControlFlowDepth;
bool mCaseInsideControlFlow;
int mDefaultCount;
std::set<int> mCasesSigned;
std::set<unsigned int> mCasesUnsigned;
bool mDuplicateCases;
};
class TDiagnostics;
class TIntermBlock;
// Check for errors and output error messages on the context.
// Returns true if there are no errors.
bool ValidateSwitchStatementList(TBasicType switchType,
TDiagnostics *diagnostics,
TIntermBlock *statementList,
const TSourceLoc &loc);
} // namespace sh
......
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