tint/resolver: Handle diagnostic directives

The Resolver parses the diagnostic rule and sets the updated severity
in a ScopeStack, which is stored in the Validator.

Automatically generate the diagnostic rule enum and its parsing logic
using intrinsics.def.

Add a "chromium_unreachable_code" diagnostic rule to test this.

Bug: tint:1809
Change-Id: Ia94db4321b8019f01d31a84da0fda25dfdf72f5c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/117566
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
James Price 2023-01-24 21:01:36 +00:00
parent 098e3d8f90
commit 81e55754e1
16 changed files with 293 additions and 5 deletions

View File

@ -1293,6 +1293,7 @@ if (tint_build_unittests) {
"resolver/const_eval_unary_op_test.cc", "resolver/const_eval_unary_op_test.cc",
"resolver/control_block_validation_test.cc", "resolver/control_block_validation_test.cc",
"resolver/dependency_graph_test.cc", "resolver/dependency_graph_test.cc",
"resolver/diagnostic_control_test.cc",
"resolver/entry_point_validation_test.cc", "resolver/entry_point_validation_test.cc",
"resolver/evaluation_stage_test.cc", "resolver/evaluation_stage_test.cc",
"resolver/f16_extension_test.cc", "resolver/f16_extension_test.cc",

View File

@ -926,6 +926,7 @@ if(TINT_BUILD_TESTS)
resolver/const_eval_unary_op_test.cc resolver/const_eval_unary_op_test.cc
resolver/control_block_validation_test.cc resolver/control_block_validation_test.cc
resolver/dependency_graph_test.cc resolver/dependency_graph_test.cc
resolver/diagnostic_control_test.cc
resolver/entry_point_validation_test.cc resolver/entry_point_validation_test.cc
resolver/evaluation_stage_test.cc resolver/evaluation_stage_test.cc
resolver/f16_extension_test.cc resolver/f16_extension_test.cc

View File

@ -38,6 +38,19 @@ const DiagnosticControl* DiagnosticControl::Clone(CloneContext* ctx) const {
return ctx->dst->create<DiagnosticControl>(src, severity, rule); return ctx->dst->create<DiagnosticControl>(src, severity, rule);
} }
diag::Severity ToSeverity(DiagnosticSeverity sc) {
switch (sc) {
case DiagnosticSeverity::kError:
return diag::Severity::Error;
case DiagnosticSeverity::kWarning:
return diag::Severity::Warning;
case DiagnosticSeverity::kInfo:
return diag::Severity::Note;
default:
return diag::Severity::InternalCompilerError;
}
}
/// ParseDiagnosticSeverity parses a DiagnosticSeverity from a string. /// ParseDiagnosticSeverity parses a DiagnosticSeverity from a string.
/// @param str the string to parse /// @param str the string to parse
/// @returns the parsed enum, or DiagnosticSeverity::kUndefined if the string could not be parsed. /// @returns the parsed enum, or DiagnosticSeverity::kUndefined if the string could not be parsed.
@ -73,4 +86,24 @@ std::ostream& operator<<(std::ostream& out, DiagnosticSeverity value) {
return out << "<unknown>"; return out << "<unknown>";
} }
/// ParseDiagnosticRule parses a DiagnosticRule from a string.
/// @param str the string to parse
/// @returns the parsed enum, or DiagnosticRule::kUndefined if the string could not be parsed.
DiagnosticRule ParseDiagnosticRule(std::string_view str) {
if (str == "chromium_unreachable_code") {
return DiagnosticRule::kChromiumUnreachableCode;
}
return DiagnosticRule::kUndefined;
}
std::ostream& operator<<(std::ostream& out, DiagnosticRule value) {
switch (value) {
case DiagnosticRule::kUndefined:
return out << "undefined";
case DiagnosticRule::kChromiumUnreachableCode:
return out << "chromium_unreachable_code";
}
return out << "<unknown>";
}
} // namespace tint::ast } // namespace tint::ast

View File

@ -28,8 +28,25 @@ const DiagnosticControl* DiagnosticControl::Clone(CloneContext* ctx) const {
return ctx->dst->create<DiagnosticControl>(src, severity, rule); return ctx->dst->create<DiagnosticControl>(src, severity, rule);
} }
diag::Severity ToSeverity(DiagnosticSeverity sc) {
switch (sc) {
case DiagnosticSeverity::kError:
return diag::Severity::Error;
case DiagnosticSeverity::kWarning:
return diag::Severity::Warning;
case DiagnosticSeverity::kInfo:
return diag::Severity::Note;
default:
return diag::Severity::InternalCompilerError;
}
}
{{ Eval "ParseEnum" (Sem.Enum "diagnostic_severity")}} {{ Eval "ParseEnum" (Sem.Enum "diagnostic_severity")}}
{{ Eval "EnumOStream" (Sem.Enum "diagnostic_severity")}} {{ Eval "EnumOStream" (Sem.Enum "diagnostic_severity")}}
{{ Eval "ParseEnum" (Sem.Enum "diagnostic_rule")}}
{{ Eval "EnumOStream" (Sem.Enum "diagnostic_rule")}}
} // namespace tint::ast } // namespace tint::ast

View File

@ -61,6 +61,29 @@ constexpr const char* kDiagnosticSeverityStrings[] = {
"warning", "warning",
}; };
/// The diagnostic rule.
enum class DiagnosticRule {
kUndefined,
kChromiumUnreachableCode,
};
/// @param out the std::ostream to write to
/// @param value the DiagnosticRule
/// @returns `out` so calls can be chained
std::ostream& operator<<(std::ostream& out, DiagnosticRule value);
/// ParseDiagnosticRule parses a DiagnosticRule from a string.
/// @param str the string to parse
/// @returns the parsed enum, or DiagnosticRule::kUndefined if the string could not be parsed.
DiagnosticRule ParseDiagnosticRule(std::string_view str);
constexpr const char* kDiagnosticRuleStrings[] = {
"chromium_unreachable_code",
};
/// Convert a DiagnosticSeverity to the corresponding diag::Severity.
diag::Severity ToSeverity(DiagnosticSeverity sc);
/// A diagnostic control used for diagnostic directives and attributes. /// A diagnostic control used for diagnostic directives and attributes.
class DiagnosticControl : public Castable<DiagnosticControl, Node> { class DiagnosticControl : public Castable<DiagnosticControl, Node> {
public: public:

View File

@ -28,6 +28,12 @@ namespace tint::ast {
/// The diagnostic severity control. /// The diagnostic severity control.
{{ Eval "DeclareEnum" (Sem.Enum "diagnostic_severity") }} {{ Eval "DeclareEnum" (Sem.Enum "diagnostic_severity") }}
/// The diagnostic rule.
{{ Eval "DeclareEnum" (Sem.Enum "diagnostic_rule") }}
/// Convert a DiagnosticSeverity to the corresponding diag::Severity.
diag::Severity ToSeverity(DiagnosticSeverity sc);
/// A diagnostic control used for diagnostic directives and attributes. /// A diagnostic control used for diagnostic directives and attributes.
class DiagnosticControl : public Castable<DiagnosticControl, Node> { class DiagnosticControl : public Castable<DiagnosticControl, Node> {
public: public:

View File

@ -46,5 +46,21 @@ void DiagnosticSeverityParser(::benchmark::State& state) {
BENCHMARK(DiagnosticSeverityParser); BENCHMARK(DiagnosticSeverityParser);
void DiagnosticRuleParser(::benchmark::State& state) {
std::array kStrings{
"hromium_unyeachable_code", "chrorrillmGunnreachable_c77de", "chromium_unreachable4cod00",
"chromium_unreachable_code", "chromium_unracaboo_code", "chromium_unrzzchabl_code",
"ciipp11ium_unreachable_cod",
};
for (auto _ : state) {
for (auto& str : kStrings) {
auto result = ParseDiagnosticRule(str);
benchmark::DoNotOptimize(result);
}
}
}
BENCHMARK(DiagnosticRuleParser);
} // namespace } // namespace
} // namespace tint::ast } // namespace tint::ast

View File

@ -21,5 +21,7 @@ namespace {
{{ Eval "BenchmarkParseEnum" (Sem.Enum "diagnostic_severity")}} {{ Eval "BenchmarkParseEnum" (Sem.Enum "diagnostic_severity")}}
{{ Eval "BenchmarkParseEnum" (Sem.Enum "diagnostic_rule")}}
} // namespace } // namespace
} // namespace tint::ast } // namespace tint::ast

View File

@ -100,5 +100,53 @@ INSTANTIATE_TEST_SUITE_P(ValidCases, DiagnosticSeverityPrintTest, testing::Value
} // namespace diagnostic_severity_tests } // namespace diagnostic_severity_tests
namespace diagnostic_rule_tests {
namespace parse_print_tests {
struct Case {
const char* string;
DiagnosticRule value;
};
inline std::ostream& operator<<(std::ostream& out, Case c) {
return out << "'" << std::string(c.string) << "'";
}
static constexpr Case kValidCases[] = {
{"chromium_unreachable_code", DiagnosticRule::kChromiumUnreachableCode},
};
static constexpr Case kInvalidCases[] = {
{"cXromggum_unreachable_cde", DiagnosticRule::kUndefined},
{"chroVium_unruchble_codX", DiagnosticRule::kUndefined},
{"chromium_3nreachable_code", DiagnosticRule::kUndefined},
};
using DiagnosticRuleParseTest = testing::TestWithParam<Case>;
TEST_P(DiagnosticRuleParseTest, Parse) {
const char* string = GetParam().string;
DiagnosticRule expect = GetParam().value;
EXPECT_EQ(expect, ParseDiagnosticRule(string));
}
INSTANTIATE_TEST_SUITE_P(ValidCases, DiagnosticRuleParseTest, testing::ValuesIn(kValidCases));
INSTANTIATE_TEST_SUITE_P(InvalidCases, DiagnosticRuleParseTest, testing::ValuesIn(kInvalidCases));
using DiagnosticRulePrintTest = testing::TestWithParam<Case>;
TEST_P(DiagnosticRulePrintTest, Print) {
DiagnosticRule value = GetParam().value;
const char* expect = GetParam().string;
EXPECT_EQ(expect, utils::ToString(value));
}
INSTANTIATE_TEST_SUITE_P(ValidCases, DiagnosticRulePrintTest, testing::ValuesIn(kValidCases));
} // namespace parse_print_tests
} // namespace diagnostic_rule_tests
} // namespace } // namespace
} // namespace tint::ast } // namespace tint::ast

View File

@ -41,5 +41,11 @@ namespace diagnostic_severity_tests {
} // namespace diagnostic_severity_tests } // namespace diagnostic_severity_tests
namespace diagnostic_rule_tests {
{{ Eval "TestParsePrintEnum" (Sem.Enum "diagnostic_rule")}}
} // namespace diagnostic_rule_tests
} // namespace } // namespace
} // namespace tint::ast } // namespace tint::ast

View File

@ -40,6 +40,12 @@ enum builtin_value {
@internal point_size @internal point_size
} }
// https://gpuweb.github.io/gpuweb/wgsl/#filterable-triggering-rules
enum diagnostic_rule {
// Chromium specific rules not defined in the spec.
chromium_unreachable_code
}
// https://gpuweb.github.io/gpuweb/wgsl/#syntax-severity_control_name // https://gpuweb.github.io/gpuweb/wgsl/#syntax-severity_control_name
enum diagnostic_severity { enum diagnostic_severity {
error error

View File

@ -0,0 +1,75 @@
// Copyright 2023 The Tint Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#include "src/tint/resolver/resolver.h"
#include "src/tint/resolver/resolver_test_helper.h"
using namespace tint::number_suffixes; // NOLINT
namespace tint::resolver {
namespace {
using ResolverDiagnosticControlTest = ResolverTest;
TEST_F(ResolverDiagnosticControlTest, UnreachableCode_DefaultSeverity) {
auto stmts = utils::Vector{Return(), Return()};
Func("foo", {}, ty.void_(), stmts);
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), R"(warning: code is unreachable)");
}
TEST_F(ResolverDiagnosticControlTest, UnreachableCode_ErrorViaDirective) {
DiagnosticDirective(ast::DiagnosticSeverity::kError, Expr("chromium_unreachable_code"));
auto stmts = utils::Vector{Return(), Return()};
Func("foo", {}, ty.void_(), stmts);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(error: code is unreachable)");
}
TEST_F(ResolverDiagnosticControlTest, UnreachableCode_WarningViaDirective) {
DiagnosticDirective(ast::DiagnosticSeverity::kWarning, Expr("chromium_unreachable_code"));
auto stmts = utils::Vector{Return(), Return()};
Func("foo", {}, ty.void_(), stmts);
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), R"(warning: code is unreachable)");
}
TEST_F(ResolverDiagnosticControlTest, UnreachableCode_InfoViaDirective) {
DiagnosticDirective(ast::DiagnosticSeverity::kInfo, Expr("chromium_unreachable_code"));
auto stmts = utils::Vector{Return(), Return()};
Func("foo", {}, ty.void_(), stmts);
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), R"(note: code is unreachable)");
}
TEST_F(ResolverDiagnosticControlTest, UnreachableCode_OffViaDirective) {
DiagnosticDirective(ast::DiagnosticSeverity::kOff, Expr("chromium_unreachable_code"));
auto stmts = utils::Vector{Return(), Return()};
Func("foo", {}, ty.void_(), stmts);
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_TRUE(r()->error().empty());
}
} // namespace
} // namespace tint::resolver

View File

@ -151,6 +151,7 @@ bool Resolver::ResolveInternal() {
Mark(decl); Mark(decl);
if (!Switch<bool>( if (!Switch<bool>(
decl, // decl, //
[&](const ast::DiagnosticControl* dc) { return DiagnosticControl(dc); },
[&](const ast::Enable* e) { return Enable(e); }, [&](const ast::Enable* e) { return Enable(e); },
[&](const ast::TypeDecl* td) { return TypeDecl(td); }, [&](const ast::TypeDecl* td) { return TypeDecl(td); },
[&](const ast::Function* func) { return Function(func); }, [&](const ast::Function* func) { return Function(func); },
@ -3050,6 +3051,16 @@ sem::Expression* Resolver::UnaryOp(const ast::UnaryOpExpression* unary) {
return sem; return sem;
} }
bool Resolver::DiagnosticControl(const ast::DiagnosticControl* control) {
Mark(control->rule_name);
auto rule_name = builder_->Symbols().NameFor(control->rule_name->symbol);
auto rule = ast::ParseDiagnosticRule(rule_name);
if (rule != ast::DiagnosticRule::kUndefined) {
validator_.DiagnosticFilters().Set(rule, control->severity);
}
return true;
}
bool Resolver::Enable(const ast::Enable* enable) { bool Resolver::Enable(const ast::Enable* enable) {
enabled_extensions_.Add(enable->extension); enabled_extensions_.Add(enable->extension);
return true; return true;

View File

@ -30,7 +30,6 @@
#include "src/tint/resolver/intrinsic_table.h" #include "src/tint/resolver/intrinsic_table.h"
#include "src/tint/resolver/sem_helper.h" #include "src/tint/resolver/sem_helper.h"
#include "src/tint/resolver/validator.h" #include "src/tint/resolver/validator.h"
#include "src/tint/scope_stack.h"
#include "src/tint/sem/binding_point.h" #include "src/tint/sem/binding_point.h"
#include "src/tint/sem/block_statement.h" #include "src/tint/sem/block_statement.h"
#include "src/tint/sem/function.h" #include "src/tint/sem/function.h"
@ -262,6 +261,10 @@ class Resolver {
/// @param ty the ast::Type /// @param ty the ast::Type
type::Type* Type(const ast::Type* ty); type::Type* Type(const ast::Type* ty);
/// @param control the diagnostic control
/// @returns true on success, false on failure
bool DiagnosticControl(const ast::DiagnosticControl* control);
/// @param enable the enable declaration /// @param enable the enable declaration
/// @returns the resolved extension /// @returns the resolved extension
bool Enable(const ast::Enable* enable); bool Enable(const ast::Enable* enable);

View File

@ -166,7 +166,11 @@ Validator::Validator(
sem_(sem), sem_(sem),
enabled_extensions_(enabled_extensions), enabled_extensions_(enabled_extensions),
atomic_composite_info_(atomic_composite_info), atomic_composite_info_(atomic_composite_info),
valid_type_storage_layouts_(valid_type_storage_layouts) {} valid_type_storage_layouts_(valid_type_storage_layouts) {
// Set default severities for filterable diagnostic rules.
diagnostic_filters_.Set(ast::DiagnosticRule::kChromiumUnreachableCode,
ast::DiagnosticSeverity::kWarning);
}
Validator::~Validator() = default; Validator::~Validator() = default;
@ -182,6 +186,24 @@ void Validator::AddNote(const std::string& msg, const Source& source) const {
diagnostics_.add_note(diag::System::Resolver, msg, source); diagnostics_.add_note(diag::System::Resolver, msg, source);
} }
bool Validator::AddDiagnostic(ast::DiagnosticRule rule,
const std::string& msg,
const Source& source) const {
auto severity = diagnostic_filters_.Get(rule);
if (severity != ast::DiagnosticSeverity::kOff) {
diag::Diagnostic d{};
d.severity = ToSeverity(severity);
d.system = diag::System::Resolver;
d.source = source;
d.message = msg;
diagnostics_.add(std::move(d));
if (severity == ast::DiagnosticSeverity::kError) {
return false;
}
}
return true;
}
// https://gpuweb.github.io/gpuweb/wgsl/#plain-types-section // https://gpuweb.github.io/gpuweb/wgsl/#plain-types-section
bool Validator::IsPlain(const type::Type* type) const { bool Validator::IsPlain(const type::Type* type) const {
return type->is_scalar() || return type->is_scalar() ||
@ -1358,9 +1380,10 @@ bool Validator::EvaluationStage(const sem::Expression* expr,
bool Validator::Statements(utils::VectorRef<const ast::Statement*> stmts) const { bool Validator::Statements(utils::VectorRef<const ast::Statement*> stmts) const {
for (auto* stmt : stmts) { for (auto* stmt : stmts) {
if (!sem_.Get(stmt)->IsReachable()) { if (!sem_.Get(stmt)->IsReachable()) {
/// TODO(https://github.com/gpuweb/gpuweb/issues/2378): This may need to if (!AddDiagnostic(ast::DiagnosticRule::kChromiumUnreachableCode, "code is unreachable",
/// become an error. stmt->source)) {
AddWarning("code is unreachable", stmt->source); return false;
}
break; break;
} }
} }

View File

@ -22,6 +22,7 @@
#include "src/tint/ast/pipeline_stage.h" #include "src/tint/ast/pipeline_stage.h"
#include "src/tint/program_builder.h" #include "src/tint/program_builder.h"
#include "src/tint/resolver/sem_helper.h" #include "src/tint/resolver/sem_helper.h"
#include "src/tint/scope_stack.h"
#include "src/tint/sem/evaluation_stage.h" #include "src/tint/sem/evaluation_stage.h"
#include "src/tint/source.h" #include "src/tint/source.h"
#include "src/tint/utils/hash.h" #include "src/tint/utils/hash.h"
@ -84,6 +85,9 @@ struct TypeAndAddressSpace {
} }
}; };
/// DiagnosticFilterStack is a scoped stack of diagnostic filters.
using DiagnosticFilterStack = ScopeStack<ast::DiagnosticRule, ast::DiagnosticSeverity>;
/// Validation logic for various ast nodes. The validations in general should /// Validation logic for various ast nodes. The validations in general should
/// be shallow and depend on the resolver to call on children. The validations /// be shallow and depend on the resolver to call on children. The validations
/// also assume that sem changes have already been made. The validation checks /// also assume that sem changes have already been made. The validation checks
@ -118,6 +122,18 @@ class Validator {
/// @param source the note source /// @param source the note source
void AddNote(const std::string& msg, const Source& source) const; void AddNote(const std::string& msg, const Source& source) const;
/// Adds the given message to the diagnostics with current severity for the given rule.
/// @param rule the diagnostic trigger rule
/// @param msg the diagnostic message
/// @param source the diagnostic source
/// @returns false if the diagnostic is an error for the given trigger rule
bool AddDiagnostic(ast::DiagnosticRule rule,
const std::string& msg,
const Source& source) const;
/// @returns the diagnostic filter stack
DiagnosticFilterStack& DiagnosticFilters() { return diagnostic_filters_; }
/// @param type the given type /// @param type the given type
/// @returns true if the given type is a plain type /// @returns true if the given type is a plain type
bool IsPlain(const type::Type* type) const; bool IsPlain(const type::Type* type) const;
@ -525,6 +541,7 @@ class Validator {
SymbolTable& symbols_; SymbolTable& symbols_;
diag::List& diagnostics_; diag::List& diagnostics_;
SemHelper& sem_; SemHelper& sem_;
DiagnosticFilterStack diagnostic_filters_;
const ast::Extensions& enabled_extensions_; const ast::Extensions& enabled_extensions_;
const utils::Hashmap<const type::Type*, const Source*, 8>& atomic_composite_info_; const utils::Hashmap<const type::Type*, const Source*, 8>& atomic_composite_info_;
utils::Hashset<TypeAndAddressSpace, 8>& valid_type_storage_layouts_; utils::Hashset<TypeAndAddressSpace, 8>& valid_type_storage_layouts_;