From 7a5d505896655554956e65d4c77db37f6f39dd98 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Fri, 25 Oct 2024 14:21:46 -0400 Subject: [PATCH] feat(wgsl-in): detect conflicting diag. filters --- naga/src/diagnostic_filter.rs | 91 ++++++++++++++++++++++++++++++++ naga/src/front/wgsl/error.rs | 32 ++++++++++- naga/src/front/wgsl/parse/mod.rs | 6 ++- 3 files changed, 127 insertions(+), 2 deletions(-) diff --git a/naga/src/diagnostic_filter.rs b/naga/src/diagnostic_filter.rs index e2683b8fa..81b7b7bc7 100644 --- a/naga/src/diagnostic_filter.rs +++ b/naga/src/diagnostic_filter.rs @@ -1,5 +1,11 @@ //! [`DiagnosticFilter`]s and supporting functionality. +use crate::Handle; +#[cfg(feature = "wgsl-in")] +use crate::Span; +#[cfg(feature = "wgsl-in")] +use indexmap::IndexMap; + /// A severity set on a [`DiagnosticFilter`]. /// /// @@ -95,3 +101,88 @@ pub struct DiagnosticFilter { pub new_severity: Severity, pub triggering_rule: FilterableTriggeringRule, } + +/// A map of diagnostic filters to their severity and first occurrence's span. +/// +/// Intended for front ends' first step into storing parsed [`DiagnosticFilter`]s. +#[derive(Clone, Debug, Default)] +#[cfg(feature = "wgsl-in")] +pub(crate) struct DiagnosticFilterMap(IndexMap); + +#[cfg(feature = "wgsl-in")] +impl DiagnosticFilterMap { + pub(crate) fn new() -> Self { + Self::default() + } + + /// Add the given `diagnostic_filter` parsed at the given `span` to this map. + pub(crate) fn add( + &mut self, + diagnostic_filter: DiagnosticFilter, + span: Span, + ) -> Result<(), ConflictingDiagnosticRuleError> { + use indexmap::map::Entry; + + let &mut Self(ref mut diagnostic_filters) = self; + let DiagnosticFilter { + new_severity, + triggering_rule, + } = diagnostic_filter; + + match diagnostic_filters.entry(triggering_rule) { + Entry::Vacant(entry) => { + entry.insert((new_severity, span)); + } + Entry::Occupied(entry) => { + let &(first_severity, first_span) = entry.get(); + if first_severity != new_severity { + return Err(ConflictingDiagnosticRuleError { + triggering_rule, + triggering_rule_spans: [first_span, span], + }); + } + } + } + Ok(()) + } +} + +/// An error returned by [`DiagnosticFilterMap::add`] when it encounters conflicting rules. +#[cfg(feature = "wgsl-in")] +#[derive(Clone, Debug)] +pub(crate) struct ConflictingDiagnosticRuleError { + pub triggering_rule: FilterableTriggeringRule, + pub triggering_rule_spans: [Span; 2], +} + +/// Represents a single parent-linking node in a tree of [`DiagnosticFilter`]s backed by a +/// [`crate::Arena`]. +/// +/// A single element of a _tree_ of diagnostic filter rules stored in +/// [`crate::Module::diagnostic_filters`]. When nodes are built by a front-end, module-applicable +/// filter rules are chained together in runs based on parse site. For instance, given the +/// following: +/// +/// - Module-applicable rules `a` and `b`. +/// - Rules `c` and `d`, applicable to an entry point called `c_and_d_func`. +/// - Rule `e`, applicable to an entry point called `e_func`. +/// +/// The tree would be represented as follows: +/// +/// ```text +/// a <- b +/// ^ +/// |- c <- d +/// | +/// \- e +/// ``` +/// +/// ...where: +/// +/// - `d` is the first leaf consulted by validation in `c_and_d_func`. +/// - `e` is the first leaf consulted by validation in `e_func`. +#[derive(Clone, Debug)] +pub struct DiagnosticFilterNode { + pub inner: DiagnosticFilter, + pub parent: Option>, +} diff --git a/naga/src/front/wgsl/error.rs b/naga/src/front/wgsl/error.rs index f5e633208..428d7bad9 100644 --- a/naga/src/front/wgsl/error.rs +++ b/naga/src/front/wgsl/error.rs @@ -1,4 +1,4 @@ -use crate::diagnostic_filter::FilterableTriggeringRule; +use crate::diagnostic_filter::ConflictingDiagnosticRuleError; use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, UnimplementedEnableExtension, }; @@ -295,12 +295,19 @@ pub(crate) enum Error<'a> { DiagnosticInvalidSeverity { severity_control_name_span: Span, }, + DiagnosticDuplicateTriggeringRule(ConflictingDiagnosticRuleError), DiagnosticNotYetImplemented { triggering_rule: FilterableTriggeringRule, span: Span, }, } +impl<'a> From for Error<'a> { + fn from(value: ConflictingDiagnosticRuleError) -> Self { + Self::DiagnosticDuplicateTriggeringRule(value) + } +} + #[derive(Clone, Debug)] pub(crate) struct AutoConversionError { pub dest_span: Span, @@ -1017,6 +1024,29 @@ 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_ident() + ), + 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 the severity is the same; ", + "delete the rule you don't want, or ", + "ensure that all severities with the same rule name match" + ) + .into()], + } + } Error::DiagnosticNotYetImplemented { triggering_rule, span, diff --git a/naga/src/front/wgsl/parse/mod.rs b/naga/src/front/wgsl/parse/mod.rs index 27f420137..ecafe53bb 100644 --- a/naga/src/front/wgsl/parse/mod.rs +++ b/naga/src/front/wgsl/parse/mod.rs @@ -1,4 +1,6 @@ -use crate::diagnostic_filter::{self, DiagnosticFilter, FilterableTriggeringRule}; +use crate::diagnostic_filter::{ + self, DiagnosticFilter, DiagnosticFilterMap, FilterableTriggeringRule, +}; use crate::front::wgsl::error::{Error, ExpectedToken}; use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, EnableExtensions, UnimplementedEnableExtension, @@ -2522,6 +2524,7 @@ impl Parser { let mut lexer = Lexer::new(source); let mut tu = ast::TranslationUnit::default(); let mut enable_extensions = EnableExtensions::empty(); + let mut diagnostic_filters = DiagnosticFilterMap::new(); // Parse directives. while let Ok((ident, _directive_ident_span)) = lexer.peek_ident_with_span() { @@ -2533,6 +2536,7 @@ impl Parser { if let Some(diagnostic_filter) = self.diagnostic_filter(&mut lexer)? { let triggering_rule = diagnostic_filter.triggering_rule; let span = self.peek_rule_span(&lexer); + diagnostic_filters.add(diagnostic_filter, span)?; Err(Error::DiagnosticNotYetImplemented { triggering_rule, span,