fix(wgsl-in): include user and unknown rules in diagnostic(…) tracking

CHANGELOG.md

@@ -90,7 +90,7 @@ There are some limitations to keep in mind with this new functionality:
 - Not all lints can be controlled with `diagnostic(…)` rules. In fact, only the `derivative_uniformity` triggering rule exists in the WGSL standard. That said, Naga contributors are excited to see how this level of control unlocks a new ecosystem of configurable diagnostics.
 - Finally, `diagnostic(…)` rules are not yet emitted in WGSL output. This means that `wgsl-in` → `wgsl-out` is currently a lossy process. We felt that it was important to unblock users who needed `diagnostic(…)` rules (i.e., <https://github.com/gfx-rs/wgpu/issues/3135>) before we took significant effort to fix this (tracked in <https://github.com/gfx-rs/wgpu/issues/6496>).
 
-By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148](https://github.com/gfx-rs/wgpu/pull/6148), [#6533](https://github.com/gfx-rs/wgpu/pull/6533), [#6353](https://github.com/gfx-rs/wgpu/pull/6353).
+By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148](https://github.com/gfx-rs/wgpu/pull/6148), [#6533](https://github.com/gfx-rs/wgpu/pull/6533), [#6353](https://github.com/gfx-rs/wgpu/pull/6353), [#6537](https://github.com/gfx-rs/wgpu/pull/6537).
 
 ### New Features
 
@@ -3482,4 +3482,4 @@ DeviceDescriptor {
 - concept of the storage hub
 - basic recording of passes and command buffers
 - submission-based lifetime tracking and command buffer recycling
-- automatic resource transitions
+- automatic resource transitions

naga/src/diagnostic_filter.rs

@@ -54,27 +54,40 @@ impl Severity {
 /// A filterable triggering rule in a [`DiagnosticFilter`].
 ///
 /// <https://www.w3.org/TR/WGSL/#filterable-triggering-rules>
-#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
 #[cfg_attr(feature = "serialize", derive(Serialize))]
 #[cfg_attr(feature = "deserialize", derive(Deserialize))]
 #[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
 pub enum FilterableTriggeringRule {
+    Standard(StandardFilterableTriggeringRule),
+    Unknown(Box<str>),
+    User(Box<[Box<str>; 2]>),
+}
+
+/// A filterable triggering rule in a [`DiagnosticFilter`].
+///
+/// <https://www.w3.org/TR/WGSL/#filterable-triggering-rules>
+#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
+#[cfg_attr(feature = "serialize", derive(Serialize))]
+#[cfg_attr(feature = "deserialize", derive(Deserialize))]
+#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
+pub enum StandardFilterableTriggeringRule {
     DerivativeUniformity,
 }
 
-impl FilterableTriggeringRule {
+impl StandardFilterableTriggeringRule {
     /// The default severity associated with this triggering rule.
     ///
     /// See <https://www.w3.org/TR/WGSL/#filterable-triggering-rules> for a table of default
     /// severities.
     pub(crate) const fn default_severity(self) -> Severity {
         match self {
-            FilterableTriggeringRule::DerivativeUniformity => Severity::Error,
+            Self::DerivativeUniformity => Severity::Error,
         }
     }
 }
 
-/// A filter that modifies how diagnostics are emitted for shaders.
+/// A filtering rule that modifies how diagnostics are emitted for shaders.
 ///
 /// <https://www.w3.org/TR/WGSL/#diagnostic-filter>
 #[derive(Clone, Debug)]
@@ -140,7 +153,7 @@ impl DiagnosticFilterMap {
             triggering_rule,
         } = diagnostic_filter;
 
-        match diagnostic_filters.entry(triggering_rule) {
+        match diagnostic_filters.entry(triggering_rule.clone()) {
             Entry::Vacant(entry) => {
                 entry.insert((new_severity, span));
             }
@@ -152,7 +165,6 @@ impl DiagnosticFilterMap {
                 };
                 if first_severity != new_severity || should_conflict_on_full_duplicate {
                     return Err(ConflictingDiagnosticRuleError {
-                        triggering_rule,
                         triggering_rule_spans: [first_span, span],
                     });
                 }
@@ -190,7 +202,6 @@ impl IntoIterator for DiagnosticFilterMap {
 #[cfg(feature = "wgsl-in")]
 #[derive(Clone, Debug)]
 pub(crate) struct ConflictingDiagnosticRuleError {
-    pub triggering_rule: FilterableTriggeringRule,
     pub triggering_rule_spans: [Span; 2],
 }
 
@@ -232,24 +243,24 @@ pub struct DiagnosticFilterNode {
 impl DiagnosticFilterNode {
     /// Finds the most specific filter rule applicable to `triggering_rule` from the chain of
     /// diagnostic filter rules in `arena`, starting with `node`, and returns its severity. If none
-    /// is found, return the value of [`FilterableTriggeringRule::default_severity`].
+    /// is found, return the value of [`StandardFilterableTriggeringRule::default_severity`].
     ///
     /// When `triggering_rule` is not applicable to this node, its parent is consulted recursively.
     pub(crate) fn search(
         node: Option<Handle<Self>>,
         arena: &Arena<Self>,
-        triggering_rule: FilterableTriggeringRule,
+        triggering_rule: StandardFilterableTriggeringRule,
     ) -> Severity {
         let mut next = node;
         while let Some(handle) = next {
             let node = &arena[handle];
             let &Self { ref inner, parent } = node;
             let &DiagnosticFilter {
-                triggering_rule: rule,
+                triggering_rule: ref rule,
                 new_severity,
             } = inner;
 
-            if rule == triggering_rule {
+            if rule == &FilterableTriggeringRule::Standard(triggering_rule) {
                 return new_severity;
             }
 

naga/src/front/wgsl/diagnostic_filter.rs

@@ -1,4 +1,8 @@
-use crate::diagnostic_filter::{FilterableTriggeringRule, Severity};
+use std::fmt::{self, Display, Formatter};
+
+use crate::diagnostic_filter::{
+    FilterableTriggeringRule, Severity, StandardFilterableTriggeringRule,
+};
 
 impl Severity {
     const ERROR: &'static str = "error";
@@ -18,21 +22,187 @@ impl Severity {
     }
 }
 
+struct DisplayFilterableTriggeringRule<'a>(&'a FilterableTriggeringRule);
+
+impl Display for DisplayFilterableTriggeringRule<'_> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let &Self(inner) = self;
+        match *inner {
+            FilterableTriggeringRule::Standard(rule) => write!(f, "{}", rule.to_wgsl_ident()),
+            FilterableTriggeringRule::Unknown(ref rule) => write!(f, "{rule}"),
+            FilterableTriggeringRule::User(ref rules) => {
+                let &[ref seg1, ref seg2] = rules.as_ref();
+                write!(f, "{seg1}.{seg2}")
+            }
+        }
+    }
+}
+
 impl FilterableTriggeringRule {
+    /// [`Display`] this rule's identifiers in WGSL.
+    pub const fn display_wgsl_ident(&self) -> impl Display + '_ {
+        DisplayFilterableTriggeringRule(self)
+    }
+}
+
+impl StandardFilterableTriggeringRule {
     const DERIVATIVE_UNIFORMITY: &'static str = "derivative_uniformity";
 
-    /// Convert from a sentinel word in WGSL into its associated [`FilterableTriggeringRule`], if possible.
+    /// Convert from a sentinel word in WGSL into its associated
+    /// [`StandardFilterableTriggeringRule`], if possible.
     pub fn from_wgsl_ident(s: &str) -> Option<Self> {
         Some(match s {
             Self::DERIVATIVE_UNIFORMITY => Self::DerivativeUniformity,
             _ => return None,
         })
     }
 
-    /// Maps this [`FilterableTriggeringRule`] into the sentinel word associated with it in WGSL.
+    /// Maps this [`StandardFilterableTriggeringRule`] into the sentinel word associated with it in
+    /// WGSL.
     pub const fn to_wgsl_ident(self) -> &'static str {
         match self {
             Self::DerivativeUniformity => Self::DERIVATIVE_UNIFORMITY,
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    mod parse_sites_not_yet_supported {
+        use crate::front::wgsl::assert_parse_err;
+
+        #[test]
+        fn user_rules() {
+            let shader = "
+fn myfunc() {
+    if (true) @diagnostic(off, my.lint) {
+        //    ^^^^^^^^^^^^^^^^^^^^^^^^^ not yet supported, should report an error
+    }
+}
+";
+            assert_parse_err(shader, "\
+error: `@diagnostic(…)` attribute(s) not yet implemented
+  ┌─ wgsl:3:15
+  │
+3 │     if (true) @diagnostic(off, my.lint) {
+  │               ^^^^^^^^^^^^^^^^^^^^^^^^^ can't use this on compound statements (yet)
+  │
+  = note: Let Naga maintainers know that you ran into this at <https://github.com/gfx-rs/wgpu/issues/5320>, so they can prioritize it!
+
+");
+        }
+
+        #[test]
+        fn unknown_rules() {
+            let shader = "
+fn myfunc() {
+	if (true) @diagnostic(off, wat_is_this) {
+		//    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ should emit a warning
+	}
+}
+";
+            assert_parse_err(shader, "\
+error: `@diagnostic(…)` attribute(s) not yet implemented
+  ┌─ wgsl:3:12
+  │
+3 │     if (true) @diagnostic(off, wat_is_this) {
+  │               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't use this on compound statements (yet)
+  │
+  = note: Let Naga maintainers know that you ran into this at <https://github.com/gfx-rs/wgpu/issues/5320>, so they can prioritize it!
+
+");
+        }
+    }
+
+    mod directive_conflict {
+        use crate::front::wgsl::assert_parse_err;
+
+        #[test]
+        fn user_rules() {
+            let shader = "
+diagnostic(off, my.lint);
+diagnostic(warning, my.lint);
+";
+            assert_parse_err(shader, "\
+error: found conflicting `diagnostic(…)` rule(s)
+  ┌─ wgsl:2:1
+  │
+2 │ diagnostic(off, my.lint);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^ first rule
+3 │ diagnostic(warning, my.lint);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second rule
+  │
+  = note: Multiple `diagnostic(…)` rules with the same rule name conflict unless they are directives and the severity is the same.
+  = note: You should delete the rule you don't want.
+
+");
+        }
+
+        #[test]
+        fn unknown_rules() {
+            let shader = "
+diagnostic(off, wat_is_this);
+diagnostic(warning, wat_is_this);
+";
+            assert_parse_err(shader, "\
+error: found conflicting `diagnostic(…)` rule(s)
+  ┌─ wgsl:2:1
+  │
+2 │ diagnostic(off, wat_is_this);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ first rule
+3 │ diagnostic(warning, wat_is_this);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second rule
+  │
+  = note: Multiple `diagnostic(…)` rules with the same rule name conflict unless they are directives and the severity is the same.
+  = note: You should delete the rule you don't want.
+
+");
+        }
+    }
+
+    mod attribute_conflict {
+        use crate::front::wgsl::assert_parse_err;
+
+        #[test]
+        fn user_rules() {
+            let shader = "
+diagnostic(off, my.lint);
+diagnostic(warning, my.lint);
+";
+            assert_parse_err(shader, "\
+error: found conflicting `diagnostic(…)` rule(s)
+  ┌─ wgsl:2:1
+  │
+2 │ diagnostic(off, my.lint);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^ first rule
+3 │ diagnostic(warning, my.lint);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second rule
+  │
+  = note: Multiple `diagnostic(…)` rules with the same rule name conflict unless they are directives and the severity is the same.
+  = note: You should delete the rule you don't want.
+
+");
+        }
+
+        #[test]
+        fn unknown_rules() {
+            let shader = "
+diagnostic(off, wat_is_this);
+diagnostic(warning, wat_is_this);
+";
+            assert_parse_err(shader, "\
+error: found conflicting `diagnostic(…)` rule(s)
+  ┌─ wgsl:2:1
+  │
+2 │ diagnostic(off, wat_is_this);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ first rule
+3 │ diagnostic(warning, wat_is_this);
+  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second rule
+  │
+  = note: Multiple `diagnostic(…)` rules with the same rule name conflict unless they are directives and the severity is the same.
+  = note: You should delete the rule you don't want.
+
+");
+        }
+    }
+}

naga/src/front/wgsl/error.rs

@@ -1034,23 +1034,19 @@ impl<'a> Error<'a> {
                 .into()],
             },
             Error::DiagnosticDuplicateTriggeringRule(ConflictingDiagnosticRuleError {
-                triggering_rule,
                 triggering_rule_spans,
             }) => {
                 let [first_span, second_span] = triggering_rule_spans;
                 ParseError {
-                    message: format!(
-                        "found conflicting `diagnostic(…)` rule(s) for `{}`",
-                        triggering_rule.to_wgsl_ident()
-                    ),
+                    message: "found conflicting `diagnostic(…)` rule(s)".into(),
                     labels: vec![
                         (first_span, "first rule".into()),
                         (second_span, "second rule".into()),
                     ],
                     notes: vec![
                         concat!(
                             "Multiple `diagnostic(…)` rules with the same rule name ",
-                            "conflict unless it is a directive and the severity is the same.",
+                            "conflict unless they are directives and the severity is the same.",
                         )
                         .into(),
                         "You should delete the rule you don't want.".into(),