diff --git a/samples/main.cc b/samples/main.cc index 6e2456f885..fe437bd45c 100644 --- a/samples/main.cc +++ b/samples/main.cc @@ -420,6 +420,9 @@ int main(int argc, const char** argv) { tint::Context ctx(std::make_unique()); + auto diag_printer = tint::diag::Printer::create(stderr, true); + tint::diag::Formatter diag_formatter; + std::unique_ptr reader; std::unique_ptr source_file; #if TINT_BUILD_WGSL_READER @@ -478,8 +481,7 @@ int main(int argc, const char** argv) { return 1; } if (!reader->Parse()) { - auto printer = tint::diag::Printer::create(stderr, true); - tint::diag::Formatter().format(reader->diagnostics(), printer.get()); + diag_formatter.format(reader->diagnostics(), diag_printer.get()); return 1; } auto mod = reader->module(); @@ -503,7 +505,7 @@ int main(int argc, const char** argv) { tint::Validator v; if (!v.Validate(&mod)) { - std::cerr << "Validation: " << v.error() << std::endl; + diag_formatter.format(v.diagnostics(), diag_printer.get()); return 1; } diff --git a/src/validator/validator.cc b/src/validator/validator.cc index e1496497c8..b40d1c3088 100644 --- a/src/validator/validator.cc +++ b/src/validator/validator.cc @@ -25,8 +25,7 @@ Validator::~Validator() = default; bool Validator::Validate(const ast::Module* module) { bool ret = impl_->Validate(module); - if (impl_->has_error()) - set_error(impl_->error()); + diags_ = impl_->diagnostics(); return ret; } diff --git a/src/validator/validator.h b/src/validator/validator.h index 631ba0e5cb..fe4467eb5d 100644 --- a/src/validator/validator.h +++ b/src/validator/validator.h @@ -22,7 +22,8 @@ #include "src/ast/expression.h" #include "src/ast/module.h" #include "src/ast/statement.h" -#include "src/validator/validator_impl.h" +#include "src/diagnostic/diagnostic.h" +#include "src/diagnostic/formatter.h" namespace tint { @@ -41,16 +42,18 @@ class Validator { bool Validate(const ast::Module* module); /// @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 - bool has_error() const { return error_.size() > 0; } - /// Sets the error string - /// @param msg the error message - void set_error(const std::string& msg) { error_ = msg; } + bool has_error() const { return diags_.contains_errors(); } + /// @returns the full list of diagnostic messages. + const diag::List& diagnostics() const { return diags_; } private: std::unique_ptr impl_; - std::string error_; + diag::List diags_; }; } // namespace tint diff --git a/src/validator/validator_function_test.cc b/src/validator/validator_function_test.cc index 6271fe3850..4a252828d4 100644 --- a/src/validator/validator_function_test.cc +++ b/src/validator/validator_function_test.cc @@ -345,8 +345,8 @@ TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Fail) { EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_FALSE(v()->Validate(mod())); EXPECT_EQ(v()->error(), - "0:0: v-0003: At least one of vertex, fragment or compute shader " - "must be present"); + "v-0003: At least one of vertex, fragment or compute shader must " + "be present"); } } // namespace diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc index d49500950f..c866eaf6c7 100644 --- a/src/validator/validator_impl.cc +++ b/src/validator/validator_impl.cc @@ -16,6 +16,7 @@ #include #include +#include #include "src/ast/call_statement.h" #include "src/ast/function.h" @@ -38,9 +39,12 @@ ValidatorImpl::ValidatorImpl() = default; ValidatorImpl::~ValidatorImpl() = default; -void ValidatorImpl::set_error(const Source& src, const std::string& msg) { - error_ += std::to_string(src.range.begin.line) + ":" + - std::to_string(src.range.begin.column) + ": " + msg; +void ValidatorImpl::add_error(const Source& src, const std::string& msg) { + diag::Diagnostic diag; + diag.severity = diag::Severity::Error; + diag.source = src; + diag.message = msg; + diags_.add(std::move(diag)); } bool ValidatorImpl::Validate(const ast::Module* module) { @@ -75,14 +79,14 @@ bool ValidatorImpl::ValidateConstructedTypes( auto* r = member->type()->UnwrapAll()->AsArray(); if (r->IsRuntimeArray()) { if (member != st->impl()->members().back()) { - set_error(member->source(), + add_error(member->source(), "v-0015: runtime arrays may only appear as the last " "member of a struct: '" + member->name() + "'"); return false; } if (!st->IsBlockDecorated()) { - set_error(member->source(), + add_error(member->source(), "v-0031: a struct containing a runtime-sized array " "must be in the 'storage' storage class: '" + st->name() + "'"); @@ -100,18 +104,18 @@ bool ValidatorImpl::ValidateGlobalVariables( const ast::VariableList& global_vars) { for (auto* var : global_vars) { if (variable_stack_.has(var->name())) { - set_error(var->source(), + add_error(var->source(), "v-0011: redeclared global identifier '" + var->name() + "'"); return false; } 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"); return false; } if (var->is_const() && !(var->storage_class() == ast::StorageClass::kNone)) { - set_error(var->source(), + add_error(var->source(), "v-global01: global constants shouldn't have a storage class"); return false; } @@ -123,7 +127,7 @@ bool ValidatorImpl::ValidateGlobalVariables( bool ValidatorImpl::ValidateFunctions(const ast::FunctionList& funcs) { for (auto* func : funcs) { if (function_stack_.has(func->name())) { - set_error(func->source(), + add_error(func->source(), "v-0016: function names must be unique '" + func->name() + "'"); return false; } @@ -145,14 +149,14 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) { if (func->IsEntryPoint()) { shader_is_present = true; if (!func->params().empty()) { - set_error(func->source(), + add_error(func->source(), "v-0023: Entry point function must accept no parameters: '" + func->name() + "'"); return false; } if (!func->return_type()->IsVoid()) { - set_error(func->source(), + add_error(func->source(), "v-0024: Entry point function must return void: '" + func->name() + "'"); return false; @@ -164,7 +168,7 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) { } } if (stage_deco_count > 1) { - set_error( + add_error( func->source(), "v-0020: only one stage decoration permitted per entry point"); return false; @@ -172,7 +176,7 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) { } } if (!shader_is_present) { - set_error(Source{}, + add_error(Source{}, "v-0003: At least one of vertex, fragment or compute shader must " "be present"); return false; @@ -194,7 +198,7 @@ bool ValidatorImpl::ValidateFunction(const ast::Function* func) { if (!current_function_->return_type()->IsVoid()) { if (!func->get_last_statement() || !func->get_last_statement()->IsReturn()) { - set_error(func->source(), + add_error(func->source(), "v-0002: non-void function must end with a return statement"); return false; } @@ -212,7 +216,7 @@ bool ValidatorImpl::ValidateReturnStatement(const ast::ReturnStatement* ret) { ret->has_value() ? ret->value()->result_type()->UnwrapAll() : &void_type; 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 " "type, returned '" + ret_type->type_name() + "', expected '" + @@ -244,14 +248,14 @@ bool ValidatorImpl::ValidateDeclStatement( if (is_global) { error_number = "v-0013: "; } - set_error(decl->source(), + add_error(decl->source(), error_number + "redeclared identifier '" + name + "'"); return false; } variable_stack_.set(name, decl->variable()); if (decl->variable()->type()->UnwrapAll()->IsArray()) { 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 " "member of a struct: '" + decl->variable()->name() + "'"); @@ -299,7 +303,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { auto* cond_type = s->condition()->result_type()->UnwrapAll(); 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 " "scalar integer type"); return false; @@ -318,7 +322,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { for (auto* selector : case_stmt->selectors()) { 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 " "type as the selector expression."); return false; @@ -330,7 +334,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { if (selector_set.count(v)) { auto v_str = selector->type()->IsU32() ? selector->AsUint()->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 " "the case selectors for a switch statement: '" + v_str + "'"); @@ -341,7 +345,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { } if (default_counter != 1) { - set_error(s->source(), + add_error(s->source(), "v-0008: switch statement must have exactly one default clause"); return false; } @@ -349,7 +353,7 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { auto* last_clause = s->body().back(); auto* last_stmt_of_last_clause = last_clause->AsCase()->body()->last(); 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 " "the last statement in last clause of a switch"); return false; @@ -378,19 +382,19 @@ bool ValidatorImpl::ValidateCallExpr(const ast::CallExpression* expr) { // TODO(sarahM0): validate intrinsics - tied with type-determiner } else { if (!function_stack_.has(func_name)) { - set_error(expr->source(), + add_error(expr->source(), "v-0005: function must be declared before use: '" + func_name + "'"); return false; } if (func_name == current_function_->name()) { - set_error(expr->source(), + add_error(expr->source(), "v-0004: recursion is not allowed: '" + func_name + "'"); return false; } } } else { - set_error(expr->source(), "Invalid function call expression"); + add_error(expr->source(), "Invalid function call expression"); return false; } @@ -424,7 +428,7 @@ bool ValidatorImpl::ValidateConstant(const ast::AssignmentStatement* assign) { auto* ident = assign->lhs()->AsIdentifier(); if (variable_stack_.get(ident->name(), &var)) { 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() + "'"); return false; } @@ -435,7 +439,7 @@ bool ValidatorImpl::ValidateConstant(const ast::AssignmentStatement* assign) { bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) { 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; } @@ -443,7 +447,7 @@ bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) { auto* rhs_result_type = a->rhs()->result_type()->UnwrapAll(); if (lhs_result_type != rhs_result_type) { // 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 '" + rhs_result_type->type_name() + "'"); return false; @@ -468,7 +472,7 @@ bool ValidatorImpl::ValidateExpression(const ast::Expression* expr) { bool ValidatorImpl::ValidateIdentifier(const ast::IdentifierExpression* ident) { ast::Variable* var; if (!variable_stack_.get(ident->name(), &var)) { - set_error(ident->source(), + add_error(ident->source(), "v-0006: '" + ident->name() + "' is not declared"); return false; } diff --git a/src/validator/validator_impl.h b/src/validator/validator_impl.h index a29df9962d..0454cf4890 100644 --- a/src/validator/validator_impl.h +++ b/src/validator/validator_impl.h @@ -27,6 +27,8 @@ #include "src/ast/return_statement.h" #include "src/ast/statement.h" #include "src/ast/variable.h" +#include "src/diagnostic/diagnostic.h" +#include "src/diagnostic/formatter.h" #include "src/scope_stack.h" namespace tint { @@ -43,16 +45,24 @@ class ValidatorImpl { /// @returns true if the validation was successful 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 - 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 - 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 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 /// @param global_vars list of global variables to check /// @returns true if the validation was successful @@ -126,7 +136,7 @@ class ValidatorImpl { const std::vector& constructed_types); private: - std::string error_; + diag::List diags_; ScopeStack variable_stack_; ScopeStack function_stack_; ast::Function* current_function_ = nullptr;