tint/validator: Check for conflicting diagnostic controls

Two diagnostic controls conflict if they have the same rule name and
different severities.

This change also allows duplicate diagnostic attributes.

Bug: tint:1809
Change-Id: I7622dd947ffa03292ad3340161688e00862d5b24
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/117801
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>

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

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;
     }

src/tint/resolver/validator.cc

@@ -2429,11 +2429,42 @@ bool Validator::IncrementDecrementStatement(const ast::IncrementDecrementStateme
 
 bool Validator::NoDuplicateAttributes(utils::VectorRef<const ast::Attribute*> attributes) const {
     utils::Hashmap<const TypeInfo*, Source, 8> seen;
+    utils::Vector<const ast::DiagnosticControl*, 8> diagnostic_controls;
     for (auto* d : attributes) {
-        auto added = seen.Add(&d->TypeInfo(), d->source);
+        if (auto* diag = d->As<ast::DiagnosticAttribute>()) {
-        if (!added && !d->Is<ast::InternalAttribute>()) {
+            // Allow duplicate diagnostic attributes, and check for conflicts later.
-            AddError("duplicate " + d->Name() + " attribute", d->source);
+            diagnostic_controls.Push(diag->control);
-            AddNote("first attribute declared here", *added.value);
+        } else {
+            auto added = seen.Add(&d->TypeInfo(), d->source);
+            if (!added && !d->Is<ast::InternalAttribute>()) {
+                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<const ast::DiagnosticControl*> 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<Symbol, const ast::DiagnosticControl*, 8> 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;
         }
     }

src/tint/resolver/validator.h

@@ -475,6 +475,13 @@ class Validator {
     /// @returns true on success, false otherwise.
     bool NoDuplicateAttributes(utils::VectorRef<const ast::Attribute*> 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<const ast::DiagnosticControl*> controls,
+                            const char* use) const;
+
     /// Validates a address space layout
     /// @param type the type to validate
     /// @param sc the address space