[Validator] Using pointers instead of refs

Change-Id: I19a1cd27b6cbbc5d5d88a46bc5dd43c66a318b7f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26004
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
Sarah Mashayekhi 2020-07-30 02:27:03 +00:00 committed by dan sinclair
parent f961850988
commit a3f9778ee6
6 changed files with 34 additions and 32 deletions

View File

@ -437,7 +437,7 @@ int main(int argc, const char** argv) {
} }
tint::Validator v; tint::Validator v;
if (!v.Validate(mod)) { if (!v.Validate(&mod)) {
std::cerr << "Validation: " << v.error() << std::endl; std::cerr << "Validation: " << v.error() << std::endl;
return 1; return 1;
} }

View File

@ -22,7 +22,7 @@ Validator::Validator() : impl_(std::make_unique<tint::ValidatorImpl>()) {}
Validator::~Validator() = default; Validator::~Validator() = default;
bool Validator::Validate(const ast::Module& module) { bool Validator::Validate(const ast::Module* module) {
bool ret = impl_->Validate(module); bool ret = impl_->Validate(module);
if (impl_->has_error()) if (impl_->has_error())

View File

@ -38,7 +38,7 @@ class Validator {
/// Runs the validator /// Runs the validator
/// @param module the module to validate /// @param module the module to validate
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool Validate(const ast::Module& module); bool Validate(const ast::Module* module);
/// @returns error messages from the validator /// @returns error messages from the validator
const std::string& error() { return error_; } const std::string& error() { return error_; }

View File

@ -25,51 +25,53 @@ void ValidatorImpl::set_error(const Source& src, const std::string& msg) {
std::to_string(src.line) + ":" + std::to_string(src.column) + ": " + msg; std::to_string(src.line) + ":" + std::to_string(src.column) + ": " + msg;
} }
bool ValidatorImpl::Validate(const ast::Module& module) { bool ValidatorImpl::Validate(const ast::Module* module) {
if (!module)
return false;
if (!CheckImports(module)) if (!CheckImports(module))
return false; return false;
if (!ValidateFunctions(module.functions())) if (!ValidateFunctions(module->functions()))
return false; return false;
return true; return true;
} }
bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) { bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) {
for (const auto& func : funcs) { for (const auto& func : funcs) {
if (!ValidateFunction(*(func.get()))) { if (!ValidateFunction(func.get())) {
return false; return false;
} }
} }
return true; return true;
} }
bool ValidatorImpl::ValidateFunction(const ast::Function& func) { bool ValidatorImpl::ValidateFunction(const ast::Function* func) {
if (!ValidateStatements(*(func.body()))) if (!ValidateStatements(func->body()))
return false; return false;
return true; return true;
} }
bool ValidatorImpl::ValidateStatements(const ast::BlockStatement& block) { bool ValidatorImpl::ValidateStatements(const ast::BlockStatement* block) {
for (const auto& stmt : block) { for (const auto& stmt : *block) {
if (!ValidateStatement(*(stmt.get()))) { if (!ValidateStatement(stmt.get())) {
return false; return false;
} }
} }
return true; return true;
} }
bool ValidatorImpl::ValidateStatement(const ast::Statement& stmt) { bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) {
if (stmt.IsAssign() && !ValidateAssign(*(stmt.AsAssign()))) if (stmt->IsAssign() && !ValidateAssign(stmt->AsAssign()))
return false; return false;
return true; return true;
} }
bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement& a) { bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement* a) {
auto* lhs_result_type = a.lhs()->result_type()->UnwrapAliasPtrAlias(); auto* lhs_result_type = a->lhs()->result_type()->UnwrapAliasPtrAlias();
auto* rhs_result_type = a.rhs()->result_type()->UnwrapAliasPtrAlias(); auto* rhs_result_type = a->rhs()->result_type()->UnwrapAliasPtrAlias();
if (lhs_result_type != rhs_result_type) { if (lhs_result_type != rhs_result_type) {
// TODO(sarahM0): figur out what should be the error number. // TODO(sarahM0): figur out what should be the error number.
set_error(a.source(), "v-000x: invalid assignment of '" + set_error(a->source(), "v-000x: invalid assignment of '" +
lhs_result_type->type_name() + "' to '" + lhs_result_type->type_name() + "' to '" +
rhs_result_type->type_name() + "'"); rhs_result_type->type_name() + "'");
return false; return false;
@ -77,8 +79,8 @@ bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement& a) {
return true; return true;
} }
bool ValidatorImpl::CheckImports(const ast::Module& module) { bool ValidatorImpl::CheckImports(const ast::Module* module) {
for (const auto& import : module.imports()) { for (const auto& import : module->imports()) {
if (import->path() != "GLSL.std.450") { if (import->path() != "GLSL.std.450") {
set_error(import->source(), "v-0001: unknown import: " + import->path()); set_error(import->source(), "v-0001: unknown import: " + import->path());
return false; return false;

View File

@ -34,7 +34,7 @@ class ValidatorImpl {
/// Runs the validator /// Runs the validator
/// @param module the module to validate /// @param module the module to validate
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool Validate(const ast::Module& module); bool Validate(const ast::Module* module);
/// @returns error messages from the validator /// @returns error messages from the validator
const std::string& error() { return error_; } const std::string& error() { return error_; }
@ -53,23 +53,23 @@ class ValidatorImpl {
/// Validates a function /// Validates a function
/// @param func the function to check /// @param func the function to check
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool ValidateFunction(const ast::Function& func); bool ValidateFunction(const ast::Function* func);
/// Validates a block of statements /// Validates a block of statements
/// @param block the statements to check /// @param block the statements to check
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool ValidateStatements(const ast::BlockStatement& block); bool ValidateStatements(const ast::BlockStatement* block);
/// Validates a statement /// Validates a statement
/// @param stmt the statement to check /// @param stmt the statement to check
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool ValidateStatement(const ast::Statement& stmt); bool ValidateStatement(const ast::Statement* stmt);
/// Validates an assignment /// Validates an assignment
/// @param a the assignment to check /// @param a the assignment to check
/// @returns true if the validation was successful /// @returns true if the validation was successful
bool ValidateAssign(const ast::AssignmentStatement& a); bool ValidateAssign(const ast::AssignmentStatement* a);
/// Validates v-0001: Only allowed import is "GLSL.std.450" /// Validates v-0001: Only allowed import is "GLSL.std.450"
/// @param module the modele to check imports /// @param module the modele to check imports
/// @returns ture if input complies with v-0001 rule /// @returns ture if input complies with v-0001 rule
bool CheckImports(const ast::Module& module); bool CheckImports(const ast::Module* module);
private: private:
std::string error_; std::string error_;

View File

@ -73,7 +73,7 @@ TEST_F(ValidatorTest, Import) {
m.AddImport(std::make_unique<ast::Import>("GLSL.std.450", "glsl")); m.AddImport(std::make_unique<ast::Import>("GLSL.std.450", "glsl"));
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_TRUE(v.CheckImports(m)); EXPECT_TRUE(v.CheckImports(&m));
} }
TEST_F(ValidatorTest, Import_Fail_NotGLSL) { TEST_F(ValidatorTest, Import_Fail_NotGLSL) {
@ -82,7 +82,7 @@ TEST_F(ValidatorTest, Import_Fail_NotGLSL) {
std::make_unique<ast::Import>(Source{12, 34}, "not.GLSL", "glsl")); std::make_unique<ast::Import>(Source{12, 34}, "not.GLSL", "glsl"));
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_FALSE(v.CheckImports(m)); EXPECT_FALSE(v.CheckImports(&m));
ASSERT_TRUE(v.has_error()); ASSERT_TRUE(v.has_error());
EXPECT_EQ(v.error(), "12:34: v-0001: unknown import: not.GLSL"); EXPECT_EQ(v.error(), "12:34: v-0001: unknown import: not.GLSL");
} }
@ -93,7 +93,7 @@ TEST_F(ValidatorTest, Import_Fail_Typo) {
std::make_unique<ast::Import>(Source{12, 34}, "GLSL.std.4501", "glsl")); std::make_unique<ast::Import>(Source{12, 34}, "GLSL.std.4501", "glsl"));
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_FALSE(v.CheckImports(m)); EXPECT_FALSE(v.CheckImports(&m));
ASSERT_TRUE(v.has_error()); ASSERT_TRUE(v.has_error());
EXPECT_EQ(v.error(), "12:34: v-0001: unknown import: GLSL.std.4501"); EXPECT_EQ(v.error(), "12:34: v-0001: unknown import: GLSL.std.4501");
} }
@ -136,7 +136,7 @@ TEST_F(ValidatorTest, AssignIncompatibleTypes_Fail) {
ASSERT_NE(rhs_ptr->result_type(), nullptr); ASSERT_NE(rhs_ptr->result_type(), nullptr);
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_FALSE(v.ValidateAssign(assign)); EXPECT_FALSE(v.ValidateAssign(&assign));
ASSERT_TRUE(v.has_error()); ASSERT_TRUE(v.has_error());
// TODO(sarahM0): figure out what should be the error number. // TODO(sarahM0): figure out what should be the error number.
EXPECT_EQ(v.error(), EXPECT_EQ(v.error(),
@ -159,7 +159,7 @@ TEST_F(ValidatorTest, AssignCompatibleTypes_Pass) {
td()->RegisterVariableForTesting(&var); td()->RegisterVariableForTesting(&var);
EXPECT_TRUE(td()->DetermineResultType(&assign)) << td()->error(); EXPECT_TRUE(td()->DetermineResultType(&assign)) << td()->error();
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_TRUE(v.ValidateAssign(assign)); EXPECT_TRUE(v.ValidateAssign(&assign));
} }
TEST_F(ValidatorTest, DISABLED_AssignToConstant_Fail) { TEST_F(ValidatorTest, DISABLED_AssignToConstant_Fail) {