diff --git a/src/tint/resolver/diagnostic_control_test.cc b/src/tint/resolver/diagnostic_control_test.cc index 77ae66343f..568dc23ab7 100644 --- a/src/tint/resolver/diagnostic_control_test.cc +++ b/src/tint/resolver/diagnostic_control_test.cc @@ -202,5 +202,97 @@ Did you mean 'chromium_unreachable_code'? Possible values: 'chromium_unreachable_code', 'derivative_uniformity')"); } +TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameSameSeverity_Directive) { + DiagnosticDirective(ast::DiagnosticSeverity::kError, + Expr(Source{{12, 34}}, "chromium_unreachable_code")); + DiagnosticDirective(ast::DiagnosticSeverity::kError, + Expr(Source{{56, 78}}, "chromium_unreachable_code")); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameDifferentSeverity_Directive) { + DiagnosticDirective(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_code")); + DiagnosticDirective(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_code")); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(56:78 error: conflicting diagnostic directive +12:34 note: severity of 'chromium_unreachable_code' set to 'off' here)"); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_SameUnknownNameDifferentSeverity_Directive) { + DiagnosticDirective(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_codes")); + DiagnosticDirective(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_codes")); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(warning: unrecognized diagnostic rule 'chromium_unreachable_codes' +Did you mean 'chromium_unreachable_code'? +Possible values: 'chromium_unreachable_code', 'derivative_uniformity' +warning: unrecognized diagnostic rule 'chromium_unreachable_codes' +Did you mean 'chromium_unreachable_code'? +Possible values: 'chromium_unreachable_code', 'derivative_uniformity' +56:78 error: conflicting diagnostic directive +12:34 note: severity of 'chromium_unreachable_codes' set to 'off' here)"); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_DifferentUnknownNameDifferentSeverity_Directive) { + DiagnosticDirective(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_codes")); + DiagnosticDirective(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_codex")); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameSameSeverity_Attribute) { + auto* attr1 = DiagnosticAttribute(ast::DiagnosticSeverity::kError, + Expr(Source{{12, 34}}, "chromium_unreachable_code")); + auto* attr2 = DiagnosticAttribute(ast::DiagnosticSeverity::kError, + Expr(Source{{56, 78}}, "chromium_unreachable_code")); + Func("foo", {}, ty.void_(), {}, utils::Vector{attr1, attr2}); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_SameNameDifferentSeverity_Attribute) { + auto* attr1 = DiagnosticAttribute(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_code")); + auto* attr2 = DiagnosticAttribute(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_code")); + Func("foo", {}, ty.void_(), {}, utils::Vector{attr1, attr2}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(56:78 error: conflicting diagnostic attribute +12:34 note: severity of 'chromium_unreachable_code' set to 'off' here)"); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_SameUnknownNameDifferentSeverity_Attribute) { + auto* attr1 = DiagnosticAttribute(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_codes")); + auto* attr2 = DiagnosticAttribute(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_codes")); + Func("foo", {}, ty.void_(), {}, utils::Vector{attr1, attr2}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(warning: unrecognized diagnostic rule 'chromium_unreachable_codes' +Did you mean 'chromium_unreachable_code'? +Possible values: 'chromium_unreachable_code', 'derivative_uniformity' +warning: unrecognized diagnostic rule 'chromium_unreachable_codes' +Did you mean 'chromium_unreachable_code'? +Possible values: 'chromium_unreachable_code', 'derivative_uniformity' +56:78 error: conflicting diagnostic attribute +12:34 note: severity of 'chromium_unreachable_codes' set to 'off' here)"); +} + +TEST_F(ResolverDiagnosticControlTest, Conflict_DifferentUnknownNameDifferentSeverity_Attribute) { + auto* attr1 = DiagnosticAttribute(Source{{12, 34}}, ast::DiagnosticSeverity::kError, + Expr("chromium_unreachable_codes")); + auto* attr2 = DiagnosticAttribute(Source{{56, 78}}, ast::DiagnosticSeverity::kOff, + Expr("chromium_unreachable_codex")); + Func("foo", {}, ty.void_(), {}, utils::Vector{attr1, attr2}); + EXPECT_TRUE(r()->Resolve()) << r()->error(); +} + } // namespace } // namespace tint::resolver diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index b67ce615b3..1e777a7951 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -183,6 +183,10 @@ bool Resolver::ResolveInternal() { SetShadows(); + if (!validator_.DiagnosticControls(builder_->AST().DiagnosticControls(), "directive")) { + return false; + } + if (!validator_.PipelineStages(entry_points_)) { return false; } diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 57a2e3a79b..bba2f37b7b 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -2429,11 +2429,42 @@ bool Validator::IncrementDecrementStatement(const ast::IncrementDecrementStateme bool Validator::NoDuplicateAttributes(utils::VectorRef attributes) const { utils::Hashmap seen; + utils::Vector diagnostic_controls; for (auto* d : attributes) { - auto added = seen.Add(&d->TypeInfo(), d->source); - if (!added && !d->Is()) { - AddError("duplicate " + d->Name() + " attribute", d->source); - AddNote("first attribute declared here", *added.value); + if (auto* diag = d->As()) { + // Allow duplicate diagnostic attributes, and check for conflicts later. + diagnostic_controls.Push(diag->control); + } else { + auto added = seen.Add(&d->TypeInfo(), d->source); + if (!added && !d->Is()) { + AddError("duplicate " + d->Name() + " attribute", d->source); + AddNote("first attribute declared here", *added.value); + return false; + } + } + } + return DiagnosticControls(diagnostic_controls, "attribute"); +} + +bool Validator::DiagnosticControls(utils::VectorRef controls, + const char* use) const { + // Make sure that no two diagnostic controls conflict. + // They conflict if the rule name is the same and the severity is different. + utils::Hashmap diagnostics; + for (auto* dc : controls) { + auto diag_added = diagnostics.Add(dc->rule_name->symbol, dc); + if (!diag_added && (*diag_added.value)->severity != dc->severity) { + { + std::ostringstream ss; + ss << "conflicting diagnostic " << use; + AddError(ss.str(), dc->source); + } + { + std::ostringstream ss; + ss << "severity of '" << symbols_.NameFor(dc->rule_name->symbol) << "' set to '" + << dc->severity << "' here"; + AddNote(ss.str(), (*diag_added.value)->source); + } return false; } } diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h index 635a6ccfa7..6ff3473762 100644 --- a/src/tint/resolver/validator.h +++ b/src/tint/resolver/validator.h @@ -475,6 +475,13 @@ class Validator { /// @returns true on success, false otherwise. bool NoDuplicateAttributes(utils::VectorRef attributes) const; + /// Validates a set of diagnostic controls. + /// @param controls the diagnostic controls to validate + /// @param use the place where the controls are being used ("directive" or "attribute") + /// @returns true on success, false otherwise. + bool DiagnosticControls(utils::VectorRef controls, + const char* use) const; + /// Validates a address space layout /// @param type the type to validate /// @param sc the address space