From ed29b2b75872dfed27d1195fdcd65336e7a82687 Mon Sep 17 00:00:00 2001
From: Erich Gubler <erichdongubler@gmail.com>
Date: Thu, 14 Nov 2024 09:42:38 -0500
Subject: [PATCH] =?UTF-8?q?fix(wgsl-in):=20yield=20conflict=20errors=20for?=
 =?UTF-8?q?=20full=20`diagnostic(=E2=80=A6)`=20rule=20dupes=20in=20attr.?=
 =?UTF-8?q?=20pos.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 naga/src/diagnostic_filter.rs    | 19 ++++++++++++++++++-
 naga/src/front/wgsl/error.rs     | 15 ++++++++-------
 naga/src/front/wgsl/parse/mod.rs | 11 ++++++++---
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/naga/src/diagnostic_filter.rs b/naga/src/diagnostic_filter.rs
index 6f950407e..e90c636f3 100644
--- a/naga/src/diagnostic_filter.rs
+++ b/naga/src/diagnostic_filter.rs
@@ -86,6 +86,18 @@ pub struct DiagnosticFilter {
     pub triggering_rule: FilterableTriggeringRule,
 }
 
+/// Determines whether [`DiagnosticFilterMap::add`] should consider full duplicates a conflict.
+///
+/// In WGSL, directive position does not consider this case a conflict, while attribute position
+/// does.
+#[cfg(feature = "wgsl-in")]
+pub(crate) enum ShouldConflictOnFullDuplicate {
+    /// Use this for attributes in WGSL.
+    Yes,
+    /// Use this for directives in WGSL.
+    No,
+}
+
 /// A map of diagnostic filters to their severity and first occurrence's span.
 ///
 /// Intended for front ends' first step into storing parsed [`DiagnosticFilter`]s.
@@ -104,6 +116,7 @@ impl DiagnosticFilterMap {
         &mut self,
         diagnostic_filter: DiagnosticFilter,
         span: Span,
+        should_conflict_on_full_duplicate: ShouldConflictOnFullDuplicate,
     ) -> Result<(), ConflictingDiagnosticRuleError> {
         use indexmap::map::Entry;
 
@@ -119,7 +132,11 @@ impl DiagnosticFilterMap {
             }
             Entry::Occupied(entry) => {
                 let &(first_severity, first_span) = entry.get();
-                if first_severity != new_severity {
+                let should_conflict_on_full_duplicate = match should_conflict_on_full_duplicate {
+                    ShouldConflictOnFullDuplicate::Yes => true,
+                    ShouldConflictOnFullDuplicate::No => false,
+                };
+                if first_severity != new_severity || should_conflict_on_full_duplicate {
                     return Err(ConflictingDiagnosticRuleError {
                         triggering_rule,
                         triggering_rule_spans: [first_span, span],
diff --git a/naga/src/front/wgsl/error.rs b/naga/src/front/wgsl/error.rs
index dfe324fa8..26199b5e5 100644
--- a/naga/src/front/wgsl/error.rs
+++ b/naga/src/front/wgsl/error.rs
@@ -1047,13 +1047,14 @@ impl<'a> Error<'a> {
                         (first_span, "first rule".into()),
                         (second_span, "second rule".into()),
                     ],
-                    notes: vec![concat!(
-                        "multiple `diagnostic(…)` rules with the same rule name ",
-                        "conflict unless the severity is the same; ",
-                        "delete the rule you don't want, or ",
-                        "ensure that all severities with the same rule name match"
-                    )
-                    .into()],
+                    notes: vec![
+                        concat!(
+                            "Multiple `diagnostic(…)` rules with the same rule name ",
+                            "conflict unless it is a directive and the severity is the same.",
+                        )
+                        .into(),
+                        "You should delete the rule you don't want.".into(),
+                    ],
                 }
             }
             Error::DiagnosticAttributeNotYetImplementedAtParseSite {
diff --git a/naga/src/front/wgsl/parse/mod.rs b/naga/src/front/wgsl/parse/mod.rs
index 18261f8ad..069efa0a6 100644
--- a/naga/src/front/wgsl/parse/mod.rs
+++ b/naga/src/front/wgsl/parse/mod.rs
@@ -1,5 +1,6 @@
 use crate::diagnostic_filter::{
     self, DiagnosticFilter, DiagnosticFilterMap, DiagnosticFilterNode, FilterableTriggeringRule,
+    ShouldConflictOnFullDuplicate,
 };
 use crate::front::wgsl::error::{Error, ExpectedToken};
 use crate::front::wgsl::parse::directive::enable_extension::{
@@ -2167,7 +2168,7 @@ impl Parser {
             if let Some(DirectiveKind::Diagnostic) = DirectiveKind::from_ident(name) {
                 if let Some(filter) = self.diagnostic_filter(lexer)? {
                     let span = self.peek_rule_span(lexer);
-                    diagnostic_filters.add(filter, span)?;
+                    diagnostic_filters.add(filter, span, ShouldConflictOnFullDuplicate::Yes)?;
                 }
             } else {
                 return Err(Error::Unexpected(
@@ -2369,7 +2370,7 @@ impl Parser {
             if let Some(DirectiveKind::Diagnostic) = DirectiveKind::from_ident(name) {
                 if let Some(filter) = self.diagnostic_filter(lexer)? {
                     let span = self.peek_rule_span(lexer);
-                    diagnostic_filters.add(filter, span)?;
+                    diagnostic_filters.add(filter, span, ShouldConflictOnFullDuplicate::Yes)?;
                 }
                 continue;
             }
@@ -2602,7 +2603,11 @@ impl Parser {
                     DirectiveKind::Diagnostic => {
                         if let Some(diagnostic_filter) = self.diagnostic_filter(&mut lexer)? {
                             let span = self.peek_rule_span(&lexer);
-                            diagnostic_filters.add(diagnostic_filter, span)?;
+                            diagnostic_filters.add(
+                                diagnostic_filter,
+                                span,
+                                ShouldConflictOnFullDuplicate::No,
+                            )?;
                         }
                         lexer.expect(Token::Separator(';'))?;
                     }