Diagnostics: Add error code to the Diagnostic

Allows this to be formatted similarly to the severity.

Change-Id: I74cd863d8f1d94089ce753ab76a2c70784eb5553
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/33938
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
This commit is contained in:
Ben Clayton 2020-11-26 17:49:22 +00:00 committed by Commit Bot service account
parent 61ec48b99e
commit f32a3c1f35
9 changed files with 167 additions and 140 deletions

View File

@ -43,6 +43,9 @@ class Diagnostic {
Source source; Source source;
/// message is the text associated with the diagnostic. /// message is the text associated with the diagnostic.
std::string message; std::string message;
/// code is the error code, for example a validation error might have the code
/// `"v-0001"`.
const char* code = nullptr;
}; };
/// List is a container of Diagnostic messages. /// List is a container of Diagnostic messages.

View File

@ -16,6 +16,7 @@
#include <algorithm> #include <algorithm>
#include <sstream> #include <sstream>
#include <vector>
#include "src/diagnostic/diagnostic.h" #include "src/diagnostic/diagnostic.h"
#include "src/diagnostic/printer.h" #include "src/diagnostic/printer.h"
@ -24,38 +25,29 @@ namespace tint {
namespace diag { namespace diag {
namespace { namespace {
template <typename CharT, typename Traits> const char* to_str(Severity severity) {
std::basic_ostream<CharT, Traits>& operator<<(
std::basic_ostream<CharT, Traits>& stream,
Severity severity) {
switch (severity) { switch (severity) {
case Severity::Info: case Severity::Info:
stream << "info"; return "info";
break;
case Severity::Warning: case Severity::Warning:
stream << "warning"; return "warning";
break;
case Severity::Error: case Severity::Error:
stream << "error"; return "error";
break;
case Severity::Fatal: case Severity::Fatal:
stream << "fatal"; return "fatal";
break;
} }
return stream; return "";
} }
template <typename CharT, typename Traits> std::string to_str(const Source::Location& location) {
std::basic_ostream<CharT, Traits>& operator<<( std::stringstream ss;
std::basic_ostream<CharT, Traits>& stream,
const Source::Location& location) {
if (location.line > 0) { if (location.line > 0) {
stream << location.line; ss << location.line;
if (location.column > 0) { if (location.column > 0) {
stream << ":" << location.column; ss << ":" << location.column;
} }
} }
return stream; return ss.str();
} }
} // namespace } // namespace
@ -134,40 +126,59 @@ void Formatter::format(const List& list, Printer* printer) const {
void Formatter::format(const Diagnostic& diag, State& state) const { void Formatter::format(const Diagnostic& diag, State& state) const {
auto const& src = diag.source; auto const& src = diag.source;
auto const& rng = src.range; auto const& rng = src.range;
bool has_code = diag.code != nullptr && diag.code[0] != '\0';
state.set_style({Color::kDefault, true}); state.set_style({Color::kDefault, true});
bool emit_colon = true; struct TextAndColor {
std::string text;
Color color;
bool bold = false;
};
std::vector<TextAndColor> prefix;
prefix.reserve(6);
if (style_.print_file && src.file != nullptr && !src.file->path.empty()) { if (style_.print_file && src.file != nullptr && !src.file->path.empty()) {
state << src.file->path;
if (rng.begin.line > 0) { if (rng.begin.line > 0) {
state << ":" << rng.begin; prefix.emplace_back(TextAndColor{src.file->path + ":" + to_str(rng.begin),
Color::kDefault});
} else {
prefix.emplace_back(TextAndColor{src.file->path, Color::kDefault});
} }
} else if (rng.begin.line > 0) { } else if (rng.begin.line > 0) {
state << rng.begin; prefix.emplace_back(TextAndColor{to_str(rng.begin), Color::kDefault});
} else { }
// No position infomation was printed, so don't start the line with a colon.
emit_colon = false; Color severity_color = Color::kDefault;
switch (diag.severity) {
case Severity::Warning:
severity_color = Color::kYellow;
break;
case Severity::Error:
case Severity::Fatal:
severity_color = Color::kRed;
break;
default:
break;
} }
if (style_.print_severity) { if (style_.print_severity) {
switch (diag.severity) { prefix.emplace_back(
case Severity::Warning: TextAndColor{to_str(diag.severity), severity_color, true});
state.set_style({Color::kYellow, true}); }
break; if (has_code) {
case Severity::Error: prefix.emplace_back(TextAndColor{diag.code, severity_color});
case Severity::Fatal: }
state.set_style({Color::kRed, true});
break; for (size_t i = 0; i < prefix.size(); i++) {
default: if (i > 0) {
break; state << " ";
} }
state << " " << diag.severity << ": "; state.set_style({prefix[i].color, prefix[i].bold});
// A colon was just printed, don't repeat it. state << prefix[i].text;
emit_colon = false;
} }
state.set_style({Color::kDefault, true}); state.set_style({Color::kDefault, true});
if (emit_colon) { if (!prefix.empty()) {
state << ": "; state << ": ";
} }
state << diag.message; state << diag.message;

View File

@ -37,7 +37,8 @@ class DiagFormatterTest : public testing::Test {
Diagnostic diag_warn{Severity::Warning, Diagnostic diag_warn{Severity::Warning,
Source{Source::Range{{2, 14}, {2, 18}}, &file}, "grrr"}; Source{Source::Range{{2, 14}, {2, 18}}, &file}, "grrr"};
Diagnostic diag_err{Severity::Error, Diagnostic diag_err{Severity::Error,
Source{Source::Range{{3, 16}, {3, 21}}, &file}, "hiss"}; Source{Source::Range{{3, 16}, {3, 21}}, &file}, "hiss",
"abc123"};
Diagnostic diag_fatal{Severity::Fatal, Diagnostic diag_fatal{Severity::Fatal,
Source{Source::Range{{4, 16}, {4, 19}}, &file}, Source{Source::Range{{4, 16}, {4, 19}}, &file},
"nothing"}; "nothing"};
@ -48,7 +49,7 @@ TEST_F(DiagFormatterTest, Simple) {
auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal});
auto* expect = R"(1:14: purr auto* expect = R"(1:14: purr
2:14: grrr 2:14: grrr
3:16: hiss 3:16 abc123: hiss
4:16: nothing)"; 4:16: nothing)";
ASSERT_EQ(expect, got); ASSERT_EQ(expect, got);
} }
@ -66,7 +67,7 @@ TEST_F(DiagFormatterTest, WithFile) {
auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal});
auto* expect = R"(file.name:1:14: purr auto* expect = R"(file.name:1:14: purr
file.name:2:14: grrr file.name:2:14: grrr
file.name:3:16: hiss file.name:3:16 abc123: hiss
file.name:4:16: nothing)"; file.name:4:16: nothing)";
ASSERT_EQ(expect, got); ASSERT_EQ(expect, got);
} }
@ -76,7 +77,7 @@ TEST_F(DiagFormatterTest, WithSeverity) {
auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal});
auto* expect = R"(1:14 info: purr auto* expect = R"(1:14 info: purr
2:14 warning: grrr 2:14 warning: grrr
3:16 error: hiss 3:16 error abc123: hiss
4:16 fatal: nothing)"; 4:16 fatal: nothing)";
ASSERT_EQ(expect, got); ASSERT_EQ(expect, got);
} }
@ -92,7 +93,7 @@ the cat says meow
the dog says woof the dog says woof
^^^^ ^^^^
3:16: hiss 3:16 abc123: hiss
the snake says quack the snake says quack
^^^^^ ^^^^^
@ -114,7 +115,7 @@ file.name:2:14 warning: grrr
the dog says woof the dog says woof
^^^^ ^^^^
file.name:3:16 error: hiss file.name:3:16 error abc123: hiss
the snake says quack the snake says quack
^^^^^ ^^^^^

View File

@ -61,7 +61,7 @@ TEST_F(ValidateControlBlockTest, SwitchSelectorExpressionNoneIntegerType_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0025: switch statement selector expression must be " "12:34 v-0025: switch statement selector expression must be "
"of a scalar integer type"); "of a scalar integer type");
} }
@ -90,7 +90,7 @@ TEST_F(ValidateControlBlockTest, SwitchWithoutDefault_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0008: switch statement must have exactly one default " "12:34 v-0008: switch statement must have exactly one default "
"clause"); "clause");
} }
@ -132,7 +132,7 @@ TEST_F(ValidateControlBlockTest, SwitchWithTwoDefault_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0008: switch statement must have exactly one default " "12:34 v-0008: switch statement must have exactly one default "
"clause"); "clause");
} }
@ -168,7 +168,7 @@ TEST_F(ValidateControlBlockTest,
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0026: the case selector values must have the same " "12:34 v-0026: the case selector values must have the same "
"type as the selector expression."); "type as the selector expression.");
} }
@ -204,7 +204,7 @@ TEST_F(ValidateControlBlockTest,
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0026: the case selector values must have the same " "12:34 v-0026: the case selector values must have the same "
"type as the selector expression."); "type as the selector expression.");
} }
@ -245,7 +245,7 @@ TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueUint_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0027: a literal value must not appear more than once " "12:34 v-0027: a literal value must not appear more than once "
"in the case selectors for a switch statement: '2'"); "in the case selectors for a switch statement: '2'");
} }
@ -288,7 +288,7 @@ TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueSint_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0027: a literal value must not appear more than once in " "12:34 v-0027: a literal value must not appear more than once in "
"the case selectors for a switch statement: '10'"); "the case selectors for a switch statement: '10'");
} }
@ -317,7 +317,7 @@ TEST_F(ValidateControlBlockTest, LastClauseLastStatementIsFallthrough_Fail) {
EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(block)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(block)); EXPECT_FALSE(v()->ValidateStatements(block));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0028: a fallthrough statement must not appear as the " "12:34 v-0028: a fallthrough statement must not appear as the "
"last statement in last clause of a switch"); "last statement in last clause of a switch");
} }

View File

@ -92,9 +92,8 @@ TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatement_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( EXPECT_EQ(v()->error(),
v()->error(), "12:34 v-0002: non-void function must end with a return statement");
"12:34: v-0002: non-void function must end with a return statement");
} }
TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatementEmptyBody_Fail) { TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatementEmptyBody_Fail) {
@ -109,9 +108,8 @@ TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatementEmptyBody_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( EXPECT_EQ(v()->error(),
v()->error(), "12:34 v-0002: non-void function must end with a return statement");
"12:34: v-0002: non-void function must end with a return statement");
} }
TEST_F(ValidateFunctionTest, FunctionTypeMustMatchReturnStatementType_Pass) { TEST_F(ValidateFunctionTest, FunctionTypeMustMatchReturnStatementType_Pass) {
@ -149,7 +147,7 @@ TEST_F(ValidateFunctionTest, FunctionTypeMustMatchReturnStatementType_fail) {
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
// TODO(sarahM0): replace 000y with a rule number // TODO(sarahM0): replace 000y with a rule number
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-000y: return statement type must match its function " "12:34 v-000y: return statement type must match its function "
"return type, returned '__i32', expected '__void'"); "return type, returned '__i32', expected '__void'");
} }
@ -171,7 +169,7 @@ TEST_F(ValidateFunctionTest, FunctionTypeMustMatchReturnStatementTypeF32_fail) {
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
// TODO(sarahM0): replace 000y with a rule number // TODO(sarahM0): replace 000y with a rule number
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-000y: return statement type must match its function " "12:34 v-000y: return statement type must match its function "
"return type, returned '__i32', expected '__f32'"); "return type, returned '__i32', expected '__f32'");
} }
@ -203,8 +201,7 @@ TEST_F(ValidateFunctionTest, FunctionNamesMustBeUnique_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(), "12:34 v-0016: function names must be unique 'func'");
"12:34: v-0016: function names must be unique 'func'");
} }
TEST_F(ValidateFunctionTest, RecursionIsNotAllowed_Fail) { TEST_F(ValidateFunctionTest, RecursionIsNotAllowed_Fail) {
@ -224,7 +221,7 @@ TEST_F(ValidateFunctionTest, RecursionIsNotAllowed_Fail) {
EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_FALSE(v()->Validate(mod())) << v()->error(); EXPECT_FALSE(v()->Validate(mod())) << v()->error();
EXPECT_EQ(v()->error(), "12:34: v-0004: recursion is not allowed: 'func'"); EXPECT_EQ(v()->error(), "12:34 v-0004: recursion is not allowed: 'func'");
} }
TEST_F(ValidateFunctionTest, RecursionIsNotAllowedExpr_Fail) { TEST_F(ValidateFunctionTest, RecursionIsNotAllowedExpr_Fail) {
@ -248,7 +245,7 @@ TEST_F(ValidateFunctionTest, RecursionIsNotAllowedExpr_Fail) {
EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_FALSE(v()->Validate(mod())) << v()->error(); EXPECT_FALSE(v()->Validate(mod())) << v()->error();
EXPECT_EQ(v()->error(), "12:34: v-0004: recursion is not allowed: 'func'"); EXPECT_EQ(v()->error(), "12:34 v-0004: recursion is not allowed: 'func'");
} }
TEST_F(ValidateFunctionTest, Function_WithPipelineStage_NotVoid_Fail) { TEST_F(ValidateFunctionTest, Function_WithPipelineStage_NotVoid_Fail) {
@ -270,7 +267,7 @@ TEST_F(ValidateFunctionTest, Function_WithPipelineStage_NotVoid_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(),
"12:34: v-0024: Entry point function must return void: 'vtx_main'"); "12:34 v-0024: Entry point function must return void: 'vtx_main'");
} }
TEST_F(ValidateFunctionTest, Function_WithPipelineStage_WithParams_Fail) { TEST_F(ValidateFunctionTest, Function_WithPipelineStage_WithParams_Fail) {
@ -291,7 +288,7 @@ TEST_F(ValidateFunctionTest, Function_WithPipelineStage_WithParams_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(),
"12:34: v-0023: Entry point function must accept no parameters: " "12:34 v-0023: Entry point function must accept no parameters: "
"'vtx_func'"); "'vtx_func'");
} }
@ -314,7 +311,7 @@ TEST_F(ValidateFunctionTest, PipelineStage_MustBeUnique_Fail) {
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
EXPECT_EQ( EXPECT_EQ(
v()->error(), v()->error(),
"12:34: v-0020: only one stage decoration permitted per entry point"); "12:34 v-0020: only one stage decoration permitted per entry point");
} }
TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Pass) { TEST_F(ValidateFunctionTest, OnePipelineStageFunctionMustBePresent_Pass) {

View File

@ -39,6 +39,17 @@ ValidatorImpl::ValidatorImpl() = default;
ValidatorImpl::~ValidatorImpl() = default; ValidatorImpl::~ValidatorImpl() = default;
void ValidatorImpl::add_error(const Source& src,
const char* code,
const std::string& msg) {
diag::Diagnostic diag;
diag.severity = diag::Severity::Error;
diag.source = src;
diag.message = msg;
diag.code = code;
diags_.add(std::move(diag));
}
void ValidatorImpl::add_error(const Source& src, const std::string& msg) { void ValidatorImpl::add_error(const Source& src, const std::string& msg) {
diag::Diagnostic diag; diag::Diagnostic diag;
diag.severity = diag::Severity::Error; diag.severity = diag::Severity::Error;
@ -79,15 +90,15 @@ 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()) {
add_error(member->source(), add_error(member->source(), "v-0015",
"v-0015: runtime arrays may only appear as the last " "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()) {
add_error(member->source(), add_error(member->source(), "v-0031",
"v-0031: a struct containing a runtime-sized array " "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() + "'");
return false; return false;
@ -104,19 +115,19 @@ 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())) {
add_error(var->source(), add_error(var->source(), "v-0011",
"v-0011: redeclared global identifier '" + var->name() + "'"); "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) {
add_error(var->source(), add_error(var->source(), "v-0022",
"v-0022: global variables must have a storage class"); "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)) {
add_error(var->source(), add_error(var->source(), "v-global01",
"v-global01: global constants shouldn't have a storage class"); "global constants shouldn't have a storage class");
return false; return false;
} }
variable_stack_.set_global(var->name(), var); variable_stack_.set_global(var->name(), var);
@ -127,8 +138,8 @@ 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())) {
add_error(func->source(), add_error(func->source(), "v-0016",
"v-0016: function names must be unique '" + func->name() + "'"); "function names must be unique '" + func->name() + "'");
return false; return false;
} }
@ -149,16 +160,16 @@ 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()) {
add_error(func->source(), add_error(func->source(), "v-0023",
"v-0023: Entry point function must accept no parameters: '" + "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()) {
add_error(func->source(), add_error(
"v-0024: Entry point function must return void: '" + func->source(), "v-0024",
func->name() + "'"); "Entry point function must return void: '" + func->name() + "'");
return false; return false;
} }
auto stage_deco_count = 0; auto stage_deco_count = 0;
@ -168,16 +179,15 @@ bool ValidatorImpl::ValidateEntryPoint(const ast::FunctionList& funcs) {
} }
} }
if (stage_deco_count > 1) { if (stage_deco_count > 1) {
add_error( add_error(func->source(), "v-0020",
func->source(), "only one stage decoration permitted per entry point");
"v-0020: only one stage decoration permitted per entry point");
return false; return false;
} }
} }
} }
if (!shader_is_present) { if (!shader_is_present) {
add_error(Source{}, add_error(Source{}, "v-0003",
"v-0003: At least one of vertex, fragment or compute shader must " "At least one of vertex, fragment or compute shader must "
"be present"); "be present");
return false; return false;
} }
@ -198,8 +208,8 @@ 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()) {
add_error(func->source(), add_error(func->source(), "v-0002",
"v-0002: non-void function must end with a return statement"); "non-void function must end with a return statement");
return false; return false;
} }
} }
@ -216,8 +226,8 @@ 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()) {
add_error(ret->source(), add_error(ret->source(), "v-000y",
"v-000y: return statement type must match its function return " "return statement type must match its function return "
"type, returned '" + "type, returned '" +
ret_type->type_name() + "', expected '" + ret_type->type_name() + "', expected '" +
func_type->type_name() + "'"); func_type->type_name() + "'");
@ -244,19 +254,19 @@ bool ValidatorImpl::ValidateDeclStatement(
auto name = decl->variable()->name(); auto name = decl->variable()->name();
bool is_global = false; bool is_global = false;
if (variable_stack_.get(name, nullptr, &is_global)) { if (variable_stack_.get(name, nullptr, &is_global)) {
std::string error_number = "v-0014: "; const char* error_code = "v-0014";
if (is_global) { if (is_global) {
error_number = "v-0013: "; error_code = "v-0013";
} }
add_error(decl->source(), add_error(decl->source(), error_code,
error_number + "redeclared identifier '" + name + "'"); "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()) {
add_error(decl->source(), add_error(decl->source(), "v-0015",
"v-0015: runtime arrays may only appear as the last " "runtime arrays may only appear as the last "
"member of a struct: '" + "member of a struct: '" +
decl->variable()->name() + "'"); decl->variable()->name() + "'");
return false; return false;
@ -303,8 +313,8 @@ 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())) {
add_error(s->condition()->source(), add_error(s->condition()->source(), "v-0025",
"v-0025: switch statement selector expression must be of a " "switch statement selector expression must be of a "
"scalar integer type"); "scalar integer type");
return false; return false;
} }
@ -322,8 +332,8 @@ 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()) {
add_error(case_stmt->source(), add_error(case_stmt->source(), "v-0026",
"v-0026: the case selector values must have the same " "the case selector values must have the same "
"type as the selector expression."); "type as the selector expression.");
return false; return false;
} }
@ -334,8 +344,8 @@ 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();
add_error(case_stmt->source(), add_error(case_stmt->source(), "v-0027",
"v-0027: a literal value must not appear more than once in " "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 + "'");
return false; return false;
@ -345,16 +355,16 @@ bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
} }
if (default_counter != 1) { if (default_counter != 1) {
add_error(s->source(), add_error(s->source(), "v-0008",
"v-0008: switch statement must have exactly one default clause"); "switch statement must have exactly one default clause");
return false; return false;
} }
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()) {
add_error(last_stmt_of_last_clause->source(), add_error(last_stmt_of_last_clause->source(), "v-0028",
"v-0028: a fallthrough statement must not appear as " "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;
} }
@ -382,14 +392,13 @@ 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)) {
add_error(expr->source(), add_error(expr->source(), "v-0005",
"v-0005: function must be declared before use: '" + "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()) {
add_error(expr->source(), add_error(expr->source(), "v-0004",
"v-0004: recursion is not allowed: '" + func_name + "'"); "recursion is not allowed: '" + func_name + "'");
return false; return false;
} }
} }
@ -428,8 +437,8 @@ 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()) {
add_error(assign->source(), "v-0021: cannot re-assign a constant: '" + add_error(assign->source(), "v-0021",
ident->name() + "'"); "cannot re-assign a constant: '" + ident->name() + "'");
return false; return false;
} }
} }
@ -447,9 +456,9 @@ 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.
add_error(a->source(), "v-000x: invalid assignment of '" + add_error(a->source(), "v-000x",
lhs_result_type->type_name() + "' to '" + "invalid assignment of '" + lhs_result_type->type_name() +
rhs_result_type->type_name() + "'"); "' to '" + rhs_result_type->type_name() + "'");
return false; return false;
} }
return true; return true;
@ -472,8 +481,8 @@ 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)) {
add_error(ident->source(), add_error(ident->source(), "v-0006",
"v-0006: '" + ident->name() + "' is not declared"); "'" + ident->name() + "' is not declared");
return false; return false;
} }
return true; return true;

View File

@ -58,6 +58,12 @@ class ValidatorImpl {
/// @returns true if an error was encountered /// @returns true if an error was encountered
bool has_error() const { return diags_.contains_errors(); } bool has_error() const { return diags_.contains_errors(); }
/// Appends an error at @p src with the code @p code and message @p msg
/// @param src the source causing the error
/// @param code the validation error code
/// @param msg the error message
void add_error(const Source& src, const char* code, const std::string& msg);
/// Appends an error at @p src with the message @p msg /// 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

View File

@ -72,7 +72,7 @@ TEST_F(ValidatorTest, DISABLED_AssignToScalar_Fail) {
// TODO(sarahM0): Invalidate assignment to scalar. // TODO(sarahM0): Invalidate assignment to scalar.
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(), "12:34: v-000x: invalid assignment"); EXPECT_EQ(v()->error(), "12:34 v-000x: invalid assignment");
} }
TEST_F(ValidatorTest, UsingUndefinedVariable_Fail) { TEST_F(ValidatorTest, UsingUndefinedVariable_Fail) {
@ -156,7 +156,7 @@ TEST_F(ValidatorTest, AssignIncompatibleTypes_Fail) {
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(),
"12:34: v-000x: invalid assignment of '__i32' to '__f32'"); "12:34 v-000x: invalid assignment of '__i32' to '__f32'");
} }
TEST_F(ValidatorTest, AssignCompatibleTypesInBlockStatement_Pass) { TEST_F(ValidatorTest, AssignCompatibleTypesInBlockStatement_Pass) {
@ -213,7 +213,7 @@ TEST_F(ValidatorTest, AssignIncompatibleTypesInBlockStatement_Fail) {
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(),
"12:34: v-000x: invalid assignment of '__i32' to '__f32'"); "12:34 v-000x: invalid assignment of '__i32' to '__f32'");
} }
TEST_F(ValidatorTest, GlobalVariableWithStorageClass_Pass) { TEST_F(ValidatorTest, GlobalVariableWithStorageClass_Pass) {
@ -237,7 +237,7 @@ TEST_F(ValidatorTest, GlobalVariableNoStorageClass_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(),
"12:34: v-0022: global variables must have a storage class"); "12:34 v-0022: global variables must have a storage class");
} }
TEST_F(ValidatorTest, GlobalConstantWithStorageClass_Fail) { TEST_F(ValidatorTest, GlobalConstantWithStorageClass_Fail) {
// const<in> gloabl_var: f32; // const<in> gloabl_var: f32;
@ -252,7 +252,7 @@ TEST_F(ValidatorTest, GlobalConstantWithStorageClass_Fail) {
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
EXPECT_EQ( EXPECT_EQ(
v()->error(), v()->error(),
"12:34: v-global01: global constants shouldn't have a storage class"); "12:34 v-global01: global constants shouldn't have a storage class");
} }
TEST_F(ValidatorTest, GlobalConstNoStorageClass_Pass) { TEST_F(ValidatorTest, GlobalConstNoStorageClass_Pass) {
@ -294,7 +294,7 @@ TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Fail) {
mod()->AddFunction(func); mod()->AddFunction(func);
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
EXPECT_EQ(v()->error(), "12:34: v-0006: 'not_global_var' is not declared"); EXPECT_EQ(v()->error(), "12:34 v-0006: 'not_global_var' is not declared");
} }
TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) { TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
@ -361,7 +361,7 @@ TEST_F(ValidatorTest, UsingUndefinedVariableInnerScope_Fail) {
ASSERT_NE(lhs->result_type(), nullptr); ASSERT_NE(lhs->result_type(), nullptr);
ASSERT_NE(rhs->result_type(), nullptr); ASSERT_NE(rhs->result_type(), nullptr);
EXPECT_FALSE(v()->ValidateStatements(outer_body)); EXPECT_FALSE(v()->ValidateStatements(outer_body));
EXPECT_EQ(v()->error(), "12:34: v-0006: 'a' is not declared"); EXPECT_EQ(v()->error(), "12:34 v-0006: 'a' is not declared");
} }
TEST_F(ValidatorTest, UsingUndefinedVariableOuterScope_Pass) { TEST_F(ValidatorTest, UsingUndefinedVariableOuterScope_Pass) {
@ -437,7 +437,7 @@ TEST_F(ValidatorTest, GlobalVariableNotUnique_Fail) {
EXPECT_FALSE(v()->ValidateGlobalVariables(mod()->global_variables())); EXPECT_FALSE(v()->ValidateGlobalVariables(mod()->global_variables()));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0011: redeclared global identifier 'global_var'"); "12:34 v-0011: redeclared global identifier 'global_var'");
} }
TEST_F(ValidatorTest, AssignToConstant_Fail) { TEST_F(ValidatorTest, AssignToConstant_Fail) {
@ -465,7 +465,7 @@ TEST_F(ValidatorTest, AssignToConstant_Fail) {
ASSERT_NE(rhs->result_type(), nullptr); ASSERT_NE(rhs->result_type(), nullptr);
EXPECT_FALSE(v()->ValidateStatements(body)); EXPECT_FALSE(v()->ValidateStatements(body));
EXPECT_EQ(v()->error(), "12:34: v-0021: cannot re-assign a constant: 'a'"); EXPECT_EQ(v()->error(), "12:34 v-0021: cannot re-assign a constant: 'a'");
} }
TEST_F(ValidatorTest, GlobalVariableFunctionVariableNotUnique_Fail) { TEST_F(ValidatorTest, GlobalVariableFunctionVariableNotUnique_Fail) {
@ -497,7 +497,7 @@ TEST_F(ValidatorTest, GlobalVariableFunctionVariableNotUnique_Fail) {
EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_TRUE(td()->DetermineFunction(func)) << td()->error(); EXPECT_TRUE(td()->DetermineFunction(func)) << td()->error();
EXPECT_FALSE(v()->Validate(mod())) << v()->error(); EXPECT_FALSE(v()->Validate(mod())) << v()->error();
EXPECT_EQ(v()->error(), "12:34: v-0013: redeclared identifier 'a'"); EXPECT_EQ(v()->error(), "12:34 v-0013: redeclared identifier 'a'");
} }
TEST_F(ValidatorTest, RedeclaredIndentifier_Fail) { TEST_F(ValidatorTest, RedeclaredIndentifier_Fail) {
@ -529,7 +529,7 @@ TEST_F(ValidatorTest, RedeclaredIndentifier_Fail) {
EXPECT_TRUE(td()->Determine()) << td()->error(); EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_TRUE(td()->DetermineFunction(func)) << td()->error(); EXPECT_TRUE(td()->DetermineFunction(func)) << td()->error();
EXPECT_FALSE(v()->Validate(mod())); EXPECT_FALSE(v()->Validate(mod()));
EXPECT_EQ(v()->error(), "12:34: v-0014: redeclared identifier 'a'"); EXPECT_EQ(v()->error(), "12:34 v-0014: redeclared identifier 'a'");
} }
TEST_F(ValidatorTest, RedeclaredIdentifierInnerScope_Pass) { TEST_F(ValidatorTest, RedeclaredIdentifierInnerScope_Pass) {
@ -592,7 +592,7 @@ TEST_F(ValidatorTest, DISABLED_RedeclaredIdentifierInnerScope_False) {
EXPECT_TRUE(td()->DetermineStatements(outer_body)) << td()->error(); EXPECT_TRUE(td()->DetermineStatements(outer_body)) << td()->error();
EXPECT_FALSE(v()->ValidateStatements(outer_body)); EXPECT_FALSE(v()->ValidateStatements(outer_body));
EXPECT_EQ(v()->error(), "12:34: v-0014: redeclared identifier 'a'"); EXPECT_EQ(v()->error(), "12:34 v-0014: redeclared identifier 'a'");
} }
TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) { TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) {

View File

@ -88,7 +88,7 @@ TEST_F(ValidatorTypeTest, RuntimeArrayIsLastNoBlock_Fail) {
mod()->AddConstructedType(&struct_type); mod()->AddConstructedType(&struct_type);
EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types())); EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0031: a struct containing a runtime-sized array must be " "12:34 v-0031: a struct containing a runtime-sized array must be "
"in the 'storage' storage class: 'Foo'"); "in the 'storage' storage class: 'Foo'");
} }
@ -119,7 +119,7 @@ TEST_F(ValidatorTypeTest, RuntimeArrayIsNotLast_Fail) {
mod()->AddConstructedType(&struct_type); mod()->AddConstructedType(&struct_type);
EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types())); EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0015: runtime arrays may only appear as the last member " "12:34 v-0015: runtime arrays may only appear as the last member "
"of a struct: 'rt'"); "of a struct: 'rt'");
} }
@ -153,7 +153,7 @@ TEST_F(ValidatorTypeTest, AliasRuntimeArrayIsNotLast_Fail) {
mod()->AddConstructedType(&struct_type); mod()->AddConstructedType(&struct_type);
EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types())); EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
EXPECT_EQ(v()->error(), EXPECT_EQ(v()->error(),
"12:34: v-0015: runtime arrays may only appear as the last member " "12:34 v-0015: runtime arrays may only appear as the last member "
"of a struct: 'b'"); "of a struct: 'b'");
} }
@ -207,7 +207,7 @@ TEST_F(ValidatorTypeTest, RuntimeArrayInFunction_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(),
"12:34: v-0015: runtime arrays may only appear as the last member " "12:34 v-0015: runtime arrays may only appear as the last member "
"of a struct: 'a'"); "of a struct: 'a'");
} }