encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-06-06 22:53:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

validator: Migrate to using diagnostics

Browse Source 
Unlike error strings, diagnostics can:
* Describe more than one error
* Be printed with colors
* Highlight (`^^^`) the particular error on the line
* Can have separate severities

Change-Id: I4ead391ffbe190e55f79c5f23536a4524768478d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33820
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>
This commit is contained in:
Ben Clayton 2020-11-26 16:50:02 +00:00 committed by Commit Bot service account
parent b7b6a3f3f4
commit ba06db6e00
6 changed files with 67 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
samples/main.cc
View File

@ -420,6 +420,9 @@ int main(int argc, const char** argv) {
tint::Context ctx(std::make_unique<tint::NoopNamer>()); tint::Context ctx(std::make_unique<tint::NoopNamer>());
auto diag_printer = tint::diag::Printer::create(stderr, true);
tint::diag::Formatter diag_formatter;
std::unique_ptr<tint::reader::Reader> reader; std::unique_ptr<tint::reader::Reader> reader;
std::unique_ptr<tint::Source::File> source_file; std::unique_ptr<tint::Source::File> source_file;
#if TINT_BUILD_WGSL_READER #if TINT_BUILD_WGSL_READER
@ -478,8 +481,7 @@ int main(int argc, const char** argv) {
return 1; return 1;
} }
if (!reader->Parse()) { if (!reader->Parse()) {
auto printer = tint::diag::Printer::create(stderr, true); diag_formatter.format(reader->diagnostics(), diag_printer.get());
tint::diag::Formatter().format(reader->diagnostics(), printer.get());
return 1; return 1;
} }
auto mod = reader->module(); auto mod = reader->module();
@ -503,7 +505,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; diag_formatter.format(v.diagnostics(), diag_printer.get());
return 1; return 1;
} }

3
src/validator/validator.cc
View File

@ -25,8 +25,7 @@ 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()) diags_ = impl_->diagnostics();
set_error(impl_->error());
return ret; return ret;
} }

17
src/validator/validator.h
View File

@ -22,7 +22,8 @@
#include "src/ast/expression.h" #include "src/ast/expression.h"
#include "src/ast/module.h" #include "src/ast/module.h"
#include "src/ast/statement.h" #include "src/ast/statement.h"
#include "src/validator/validator_impl.h" #include "src/diagnostic/diagnostic.h"
#include "src/diagnostic/formatter.h"
namespace tint { namespace tint {
@ -41,16 +42,18 @@ class Validator {
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_; } std::string error() {
diag::Formatter formatter{{false, false, false}};
return formatter.format(diags_);
}
/// @returns true if an error was encountered /// @returns true if an error was encountered
bool has_error() const { return error_.size() > 0; } bool has_error() const { return diags_.contains_errors(); }
/// Sets the error string /// @returns the full list of diagnostic messages.
/// @param msg the error message const diag::List& diagnostics() const { return diags_; }
void set_error(const std::string& msg) { error_ = msg; }
private: private:
std::unique_ptr<ValidatorImpl> impl_; std::unique_ptr<ValidatorImpl> impl_;
std::string error_; diag::List diags_;
}; };
} // namespace tint } // namespace tint

4
src/validator/validator_function_test.cc
View File

@ -345,8 +345,8 @@ TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Fail) {
EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"0:0: v-0003: At least one of vertex, fragment or compute shader " "v-0003: At least one of vertex, fragment or compute shader must "
"must be present"); "be present");
} }
} // namespace } // namespace

62
src/validator/validator_impl.cc
View File

@ -16,6 +16,7 @@
#include <cassert> #include <cassert>
#include <unordered_set> #include <unordered_set>
#include <utility>
#include "src/ast/call_statement.h" #include "src/ast/call_statement.h"
#include "src/ast/function.h" #include "src/ast/function.h"
@ -38,9 +39,12 @@ ValidatorImpl::ValidatorImpl() = default;
ValidatorImpl::~ValidatorImpl() = default; ValidatorImpl::~ValidatorImpl() = default;
void ValidatorImpl::set_error(const Source& src, const std::string& msg) { void ValidatorImpl::add_error(const Source& src, const std::string& msg) {
error_ += std::to_string(src.range.begin.line) + ":" + diag::Diagnostic diag;
std::to_string(src.range.begin.column) + ": " + msg; diag.severity = diag::Severity::Error;
diag.source = src;
diag.message = msg;
diags_.add(std::move(diag));
} }
bool ValidatorImpl::Validate(const ast::Module* module) { bool ValidatorImpl::Validate(const ast::Module* module) {
@ -75,14 +79,14 @@ bool ValidatorImpl::ValidateConstructedTypes(
auto* r = member->type()->UnwrapAll()->AsArray(); auto* r = member->type()->UnwrapAll()->AsArray();
if (r->IsRuntimeArray()) { if (r->IsRuntimeArray()) {
if (member != st->impl()->members().back()) { if (member != st->impl()->members().back()) {
set_error(member->source(), add_error(member->source(),
"v-0015: runtime arrays may only appear as the last " "v-0015: runtime arrays may only appear as the last "
"member of a struct: '" + "member of a struct: '" +
member->name() + "'"); member->name() + "'");
return false; return false;
} }
if (!st->IsBlockDecorated()) { if (!st->IsBlockDecorated()) {
set_error(member->source(), add_error(member->source(),
"v-0031: a struct containing a runtime-sized array " "v-0031: a struct containing a runtime-sized array "
"must be in the 'storage' storage class: '" + "must be in the 'storage' storage class: '" +
st->name() + "'"); st->name() + "'");
@ -100,18 +104,18 @@ bool ValidatorImpl::ValidateGlobalVariables(
const ast::VariableList& global_vars) { const ast::VariableList& global_vars) {
for (auto* var : global_vars) { for (auto* var : global_vars) {
if (variable_stack_.has(var->name())) { if (variable_stack_.has(var->name())) {
set_error(var->source(), add_error(var->source(),
"v-0011: redeclared global identifier '" + var->name() + "'"); "v-0011: redeclared global identifier '" + var->name() + "'");
return false; return false;
} }
if (!var->is_const() && var->storage_class() == ast::StorageClass::kNone) { if (!var->is_const() && var->storage_class() == ast::StorageClass::kNone) {
set_error(var->source(), add_error(var->source(),
"v-0022: global variables must have a storage class"); "v-0022: global variables must have a storage class");
return false; return false;
} }
if (var->is_const() && if (var->is_const() &&
!(var->storage_class() == ast::StorageClass::kNone)) { !(var->storage_class() == ast::StorageClass::kNone)) {
set_error(var->source(), add_error(var->source(),
"v-global01: global constants shouldn't have a storage class"); "v-global01: global constants shouldn't have a storage class");
return false; return false;
} }
@ -123,7 +127,7 @@ bool ValidatorImpl::ValidateGlobalVariables(
bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) { bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) {
for (auto* func : funcs) { for (auto* func : funcs) {
if (function_stack_.has(func->name())) { if (function_stack_.has(func->name())) {
set_error(func->source(), add_error(func->source(),
"v-0016: function names must be unique '" + func->name() + "'"); "v-0016: function names must be unique '" + func->name() + "'");
return false; return false;
} }
@ -145,14 +149,14 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
if (func->IsEntryPoint()) { if (func->IsEntryPoint()) {
shader_is_present = true; shader_is_present = true;
if (!func->params().empty()) { if (!func->params().empty()) {
set_error(func->source(), add_error(func->source(),
"v-0023: Entry point function must accept no parameters: '" + "v-0023: Entry point function must accept no parameters: '" +
func->name() + "'"); func->name() + "'");
return false; return false;
} }
if (!func->return_type()->IsVoid()) { if (!func->return_type()->IsVoid()) {
set_error(func->source(), add_error(func->source(),
"v-0024: Entry point function must return void: '" + "v-0024: Entry point function must return void: '" +
func->name() + "'"); func->name() + "'");
return false; return false;
@ -164,7 +168,7 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
} }
} }
if (stage_deco_count > 1) { if (stage_deco_count > 1) {
set_error( add_error(
func->source(), func->source(),
"v-0020: only one stage decoration permitted per entry point"); "v-0020: only one stage decoration permitted per entry point");
return false; return false;
@ -172,7 +176,7 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
} }
} }
if (!shader_is_present) { if (!shader_is_present) {
set_error(Source{}, add_error(Source{},
"v-0003: At least one of vertex, fragment or compute shader must " "v-0003: At least one of vertex, fragment or compute shader must "
"be present"); "be present");
return false; return false;
@ -194,7 +198,7 @@ bool ValidatorImpl::ValidateFunction(const ast::Function* func) {
if (!current_function_->return_type()->IsVoid()) { if (!current_function_->return_type()->IsVoid()) {
if (!func->get_last_statement() || if (!func->get_last_statement() ||
!func->get_last_statement()->IsReturn()) { !func->get_last_statement()->IsReturn()) {
set_error(func->source(), add_error(func->source(),
"v-0002: non-void function must end with a return statement"); "v-0002: non-void function must end with a return statement");
return false; return false;
} }
@ -212,7 +216,7 @@ bool ValidatorImpl::ValidateReturnStatement(const ast::ReturnStatement* ret) {
ret->has_value() ? ret->value()->result_type()->UnwrapAll() : &void_type; ret->has_value() ? ret->value()->result_type()->UnwrapAll() : &void_type;
if (func_type->type_name() != ret_type->type_name()) { if (func_type->type_name() != ret_type->type_name()) {
set_error(ret->source(), add_error(ret->source(),
"v-000y: return statement type must match its function return " "v-000y: return statement type must match its function return "
"type, returned '" + "type, returned '" +
ret_type->type_name() + "', expected '" + ret_type->type_name() + "', expected '" +
@ -244,14 +248,14 @@ bool ValidatorImpl::ValidateDeclStatement(
if (is_global) { if (is_global) {
error_number = "v-0013: "; error_number = "v-0013: ";
} }
set_error(decl->source(), add_error(decl->source(),
error_number + "redeclared identifier '" + name + "'"); error_number + "redeclared identifier '" + name + "'");
return false; return false;
} }
variable_stack_.set(name, decl->variable()); variable_stack_.set(name, decl->variable());
if (decl->variable()->type()->UnwrapAll()->IsArray()) { if (decl->variable()->type()->UnwrapAll()->IsArray()) {
if (decl->variable()->type()->UnwrapAll()->AsArray()->IsRuntimeArray()) { if (decl->variable()->type()->UnwrapAll()->AsArray()->IsRuntimeArray()) {
set_error(decl->source(), add_error(decl->source(),
"v-0015: runtime arrays may only appear as the last " "v-0015: runtime arrays may only appear as the last "
"member of a struct: '" + "member of a struct: '" +
decl->variable()->name() + "'"); decl->variable()->name() + "'");
@ -299,7 +303,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
auto* cond_type = s->condition()->result_type()->UnwrapAll(); auto* cond_type = s->condition()->result_type()->UnwrapAll();
if (!(cond_type->IsI32() || cond_type->IsU32())) { if (!(cond_type->IsI32() || cond_type->IsU32())) {
set_error(s->condition()->source(), add_error(s->condition()->source(),
"v-0025: switch statement selector expression must be of a " "v-0025: switch statement selector expression must be of a "
"scalar integer type"); "scalar integer type");
return false; return false;
@ -318,7 +322,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
for (auto* selector : case_stmt->selectors()) { for (auto* selector : case_stmt->selectors()) {
if (cond_type != selector->type()) { if (cond_type != selector->type()) {
set_error(case_stmt->source(), add_error(case_stmt->source(),
"v-0026: the case selector values must have the same " "v-0026: the case selector values must have the same "
"type as the selector expression."); "type as the selector expression.");
return false; return false;
@ -330,7 +334,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
if (selector_set.count(v)) { if (selector_set.count(v)) {
auto v_str = selector->type()->IsU32() ? selector->AsUint()->to_str() auto v_str = selector->type()->IsU32() ? selector->AsUint()->to_str()
: selector->AsSint()->to_str(); : selector->AsSint()->to_str();
set_error(case_stmt->source(), add_error(case_stmt->source(),
"v-0027: a literal value must not appear more than once in " "v-0027: a literal value must not appear more than once in "
"the case selectors for a switch statement: '" + "the case selectors for a switch statement: '" +
v_str + "'"); v_str + "'");
@ -341,7 +345,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
} }
if (default_counter != 1) { if (default_counter != 1) {
set_error(s->source(), add_error(s->source(),
"v-0008: switch statement must have exactly one default clause"); "v-0008: switch statement must have exactly one default clause");
return false; return false;
} }
@ -349,7 +353,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
auto* last_clause = s->body().back(); auto* last_clause = s->body().back();
auto* last_stmt_of_last_clause = last_clause->AsCase()->body()->last(); auto* last_stmt_of_last_clause = last_clause->AsCase()->body()->last();
if (last_stmt_of_last_clause && last_stmt_of_last_clause->IsFallthrough()) { if (last_stmt_of_last_clause && last_stmt_of_last_clause->IsFallthrough()) {
set_error(last_stmt_of_last_clause->source(), add_error(last_stmt_of_last_clause->source(),
"v-0028: a fallthrough statement must not appear as " "v-0028: a fallthrough statement must not appear as "
"the last statement in last clause of a switch"); "the last statement in last clause of a switch");
return false; return false;
@ -378,19 +382,19 @@ bool ValidatorImpl::ValidateCallExpr(const ast::CallExpression* expr) {
// TODO(sarahM0): validate intrinsics - tied with type-determiner // TODO(sarahM0): validate intrinsics - tied with type-determiner
} else { } else {
if (!function_stack_.has(func_name)) { if (!function_stack_.has(func_name)) {
set_error(expr->source(), add_error(expr->source(),
"v-0005: function must be declared before use: '" + "v-0005: function must be declared before use: '" +
func_name + "'"); func_name + "'");
return false; return false;
} }
if (func_name == current_function_->name()) { if (func_name == current_function_->name()) {
set_error(expr->source(), add_error(expr->source(),
"v-0004: recursion is not allowed: '" + func_name + "'"); "v-0004: recursion is not allowed: '" + func_name + "'");
return false; return false;
} }
} }
} else { } else {
set_error(expr->source(), "Invalid function call expression"); add_error(expr->source(), "Invalid function call expression");
return false; return false;
} }
@ -424,7 +428,7 @@ bool ValidatorImpl::ValidateConstant(const ast::AssignmentStatement* assign) {
auto* ident = assign->lhs()->AsIdentifier(); auto* ident = assign->lhs()->AsIdentifier();
if (variable_stack_.get(ident->name(), &var)) { if (variable_stack_.get(ident->name(), &var)) {
if (var->is_const()) { if (var->is_const()) {
set_error(assign->source(), "v-0021: cannot re-assign a constant: '" + add_error(assign->source(), "v-0021: cannot re-assign a constant: '" +
ident->name() + "'"); ident->name() + "'");
return false; return false;
} }
@ -435,7 +439,7 @@ bool ValidatorImpl::ValidateConstant(const ast::AssignmentStatement* assign) {
bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) { bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) {
if (!a->lhs()->result_type() || !a->rhs()->result_type()) { if (!a->lhs()->result_type() || !a->rhs()->result_type()) {
set_error(a->source(), "result_type() is nullptr"); add_error(a->source(), "result_type() is nullptr");
return false; return false;
} }
@ -443,7 +447,7 @@ bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) {
auto* rhs_result_type = a->rhs()->result_type()->UnwrapAll(); auto* rhs_result_type = a->rhs()->result_type()->UnwrapAll();
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 '" + add_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;
@ -468,7 +472,7 @@ bool ValidatorImpl::ValidateExpression(const ast::Expression* expr) {
bool ValidatorImpl::ValidateIdentifier(const ast::IdentifierExpression* ident) { bool ValidatorImpl::ValidateIdentifier(const ast::IdentifierExpression* ident) {
ast::Variable* var; ast::Variable* var;
if (!variable_stack_.get(ident->name(), &var)) { if (!variable_stack_.get(ident->name(), &var)) {
set_error(ident->source(), add_error(ident->source(),
"v-0006: '" + ident->name() + "' is not declared"); "v-0006: '" + ident->name() + "' is not declared");
return false; return false;
} }

22
src/validator/validator_impl.h
View File

@ -27,6 +27,8 @@
#include "src/ast/return_statement.h" #include "src/ast/return_statement.h"
#include "src/ast/statement.h" #include "src/ast/statement.h"
#include "src/ast/variable.h" #include "src/ast/variable.h"
#include "src/diagnostic/diagnostic.h"
#include "src/diagnostic/formatter.h"
#include "src/scope_stack.h" #include "src/scope_stack.h"
namespace tint { namespace tint {
@ -43,16 +45,24 @@ class ValidatorImpl {
/// @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 the diagnostic messages
const diag::List& diagnostics() const { return diags_; }
/// @returns the diagnostic messages
diag::List& diagnostics() { return diags_; }
/// @returns error messages from the validator /// @returns error messages from the validator
const std::string& error() { return error_; } std::string error() {
diag::Formatter formatter{{false, false, false}};
return formatter.format(diags_);
}
/// @returns true if an error was encountered /// @returns true if an error was encountered
bool has_error() const { return error_.size() > 0; } bool has_error() const { return diags_.contains_errors(); }
/// Sets the error string /// Appends an error at @p src with the message @p msg
/// @param src the source causing the error /// @param src the source causing the error
/// @param msg the error message /// @param msg the error message
void set_error(const Source& src, const std::string& msg); void add_error(const Source& src, const std::string& msg);
/// Validate global variables /// Validate global variables
/// @param global_vars list of global variables to check /// @param global_vars list of global variables to check
/// @returns true if the validation was successful /// @returns true if the validation was successful
@ -126,7 +136,7 @@ class ValidatorImpl {
const std::vector<ast::type::Type*>& constructed_types); const std::vector<ast::type::Type*>& constructed_types);
private: private:
std::string error_; diag::List diags_;
ScopeStack<ast::Variable*> variable_stack_; ScopeStack<ast::Variable*> variable_stack_;
ScopeStack<ast::Function*> function_stack_; ScopeStack<ast::Function*> function_stack_;
ast::Function* current_function_ = nullptr; ast::Function* current_function_ = nullptr;