From 8c13d8ff5613007d0c5bb35ec2966d841e03b6c4 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Fri, 23 Aug 2024 12:25:08 -0400 Subject: [PATCH] =?UTF-8?q?feat(wgsl-in):=20parse=20`diagnostic(=E2=80=A6)?= =?UTF-8?q?`=20directives=20(with=20unimpl.=20err.)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++ naga/src/diagnostic_filter.rs | 97 ++++++++++++++++++++++++++ naga/src/front/wgsl/error.rs | 50 +++++++++++++ naga/src/front/wgsl/parse/directive.rs | 53 +++++++------- naga/src/front/wgsl/parse/mod.rs | 63 +++++++++++++++++ naga/src/lib.rs | 1 + 6 files changed, 243 insertions(+), 27 deletions(-) create mode 100644 naga/src/diagnostic_filter.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ef4548fa3..f1d130b0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,12 @@ Bottom level categories: ## Unreleased +## New Features + +### Naga + +- Parse `diagnostic(…)` directives, but don't implement any triggering rules yet. By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456). + ## 23.0.0 (2024-10-25) ### Themes of this release diff --git a/naga/src/diagnostic_filter.rs b/naga/src/diagnostic_filter.rs new file mode 100644 index 000000000..e2683b8fa --- /dev/null +++ b/naga/src/diagnostic_filter.rs @@ -0,0 +1,97 @@ +//! [`DiagnosticFilter`]s and supporting functionality. + +/// A severity set on a [`DiagnosticFilter`]. +/// +/// +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub enum Severity { + Off, + Info, + Warning, + Error, +} + +impl Severity { + const ERROR: &'static str = "error"; + const WARNING: &'static str = "warning"; + const INFO: &'static str = "info"; + const OFF: &'static str = "off"; + + /// Convert from a sentinel word in WGSL into its associated [`Severity`], if possible. + pub fn from_ident(s: &str) -> Option { + Some(match s { + Self::ERROR => Self::Error, + Self::WARNING => Self::Warning, + Self::INFO => Self::Info, + Self::OFF => Self::Off, + _ => return None, + }) + } + + /// Checks whether this severity is [`Self::Error`]. + /// + /// Naga does not yet support diagnostic items at lesser severities than + /// [`Severity::Error`]. When this is implemented, this method should be deleted, and the + /// severity should be used directly for reporting diagnostics. + #[cfg(feature = "wgsl-in")] + pub(crate) fn report_diag( + self, + err: E, + log_handler: impl FnOnce(E, log::Level), + ) -> Result<(), E> { + let log_level = match self { + Severity::Off => return Ok(()), + + // NOTE: These severities are not yet reported. + Severity::Info => log::Level::Info, + Severity::Warning => log::Level::Warn, + + Severity::Error => return Err(err), + }; + log_handler(err, log_level); + Ok(()) + } +} + +/// A filterable triggering rule in a [`DiagnosticFilter`]. +/// +/// +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub enum FilterableTriggeringRule { + DerivativeUniformity, +} + +impl FilterableTriggeringRule { + const DERIVATIVE_UNIFORMITY: &'static str = "derivative_uniformity"; + + /// Convert from a sentinel word in WGSL into its associated [`FilterableTriggeringRule`], if possible. + pub fn from_ident(s: &str) -> Option { + Some(match s { + Self::DERIVATIVE_UNIFORMITY => Self::DerivativeUniformity, + _ => return None, + }) + } + + /// Maps this [`FilterableTriggeringRule`] into the sentinel word associated with it in WGSL. + pub const fn to_ident(self) -> &'static str { + match self { + Self::DerivativeUniformity => Self::DERIVATIVE_UNIFORMITY, + } + } + + #[cfg(feature = "wgsl-in")] + pub(crate) const fn tracking_issue_num(self) -> u16 { + match self { + FilterableTriggeringRule::DerivativeUniformity => 5320, + } + } +} + +/// A filter that modifies how diagnostics are emitted for shaders. +/// +/// +#[derive(Clone, Debug)] +pub struct DiagnosticFilter { + pub new_severity: Severity, + pub triggering_rule: FilterableTriggeringRule, +} diff --git a/naga/src/front/wgsl/error.rs b/naga/src/front/wgsl/error.rs index 330c32270..c51f4c12f 100644 --- a/naga/src/front/wgsl/error.rs +++ b/naga/src/front/wgsl/error.rs @@ -1,3 +1,4 @@ +use crate::diagnostic_filter::FilterableTriggeringRule; use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, UnimplementedEnableExtension, }; @@ -194,6 +195,7 @@ pub(crate) enum Error<'a> { UnknownConservativeDepth(Span), UnknownEnableExtension(Span, &'a str), UnknownLanguageExtension(Span, &'a str), + UnknownDiagnosticRuleName(Span), SizeAttributeTooLow(Span, u32), AlignAttributeTooLow(Span, Alignment), NonPowerOfTwoAlignAttribute(Span), @@ -295,6 +297,13 @@ pub(crate) enum Error<'a> { kind: UnimplementedLanguageExtension, span: Span, }, + DiagnosticInvalidSeverity { + severity_control_name_span: Span, + }, + DiagnosticNotYetImplemented { + triggering_rule: FilterableTriggeringRule, + span: Span, + }, } #[derive(Clone, Debug)] @@ -562,6 +571,15 @@ impl<'a> Error<'a> { ) .into()], }, + Error::UnknownDiagnosticRuleName(span) => ParseError { + message: format!("unknown `diagnostic(…)` rule name `{}`", &source[span]), + labels: vec![(span, "not a valid diagnostic rule name".into())], + notes: vec![concat!( + "See available trigger rules at ", + "." + ) + .into()], + }, Error::SizeAttributeTooLow(bad_span, min_size) => ParseError { message: format!("struct member size must be at least {min_size}"), labels: vec![(bad_span, format!("must be at least {min_size}").into())], @@ -1008,6 +1026,38 @@ impl<'a> Error<'a> { kind.tracking_issue_num() )], }, + Error::DiagnosticInvalidSeverity { + severity_control_name_span, + } => ParseError { + message: "invalid `diagnostic(…)` severity".into(), + labels: vec![( + severity_control_name_span, + "not a valid severity level".into(), + )], + notes: vec![concat!( + "See available severities at ", + "." + ) + .into()], + }, + Error::DiagnosticNotYetImplemented { + triggering_rule, + span, + } => ParseError { + message: format!( + "the `{}` diagnostic filter is not yet supported", + triggering_rule.to_ident() + ), + labels: vec![(span, "".into())], + notes: vec![format!( + concat!( + "Let Naga maintainers know that you ran into this at ", + ", ", + "so they can prioritize it!" + ), + triggering_rule.tracking_issue_num() + )], + }, } } } diff --git a/naga/src/front/wgsl/parse/directive.rs b/naga/src/front/wgsl/parse/directive.rs index eb61ef6a4..96069ca23 100644 --- a/naga/src/front/wgsl/parse/directive.rs +++ b/naga/src/front/wgsl/parse/directive.rs @@ -8,6 +8,8 @@ pub(crate) mod language_extension; /// A parsed sentinel word indicating the type of directive to be parsed next. #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)] pub(crate) enum DirectiveKind { + /// A [`crate::diagnostic_filter`]. + Diagnostic, /// An [`enable_extension`]. Enable, /// A [`language_extension`]. @@ -23,7 +25,7 @@ impl DirectiveKind { /// Convert from a sentinel word in WGSL into its associated [`DirectiveKind`], if possible. pub fn from_ident(s: &str) -> Option { Some(match s { - Self::DIAGNOSTIC => Self::Unimplemented(UnimplementedDirectiveKind::Diagnostic), + Self::DIAGNOSTIC => Self::Diagnostic, Self::ENABLE => Self::Enable, Self::REQUIRES => Self::Requires, _ => return None, @@ -33,11 +35,10 @@ impl DirectiveKind { /// Maps this [`DirectiveKind`] into the sentinel word associated with it in WGSL. pub const fn to_ident(self) -> &'static str { match self { + Self::Diagnostic => Self::DIAGNOSTIC, Self::Enable => Self::ENABLE, Self::Requires => Self::REQUIRES, - Self::Unimplemented(kind) => match kind { - UnimplementedDirectiveKind::Diagnostic => Self::DIAGNOSTIC, - }, + Self::Unimplemented(kind) => match kind {}, } } @@ -45,7 +46,7 @@ impl DirectiveKind { fn iter() -> impl Iterator { use strum::IntoEnumIterator; - [Self::Enable, Self::Requires] + [Self::Diagnostic, Self::Enable, Self::Requires] .into_iter() .chain(UnimplementedDirectiveKind::iter().map(Self::Unimplemented)) } @@ -54,15 +55,25 @@ impl DirectiveKind { /// A [`DirectiveKind`] that is not yet implemented. See [`DirectiveKind::Unimplemented`]. #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)] #[cfg_attr(test, derive(strum::EnumIter))] -pub(crate) enum UnimplementedDirectiveKind { - Diagnostic, -} +pub(crate) enum UnimplementedDirectiveKind {} impl UnimplementedDirectiveKind { pub const fn tracking_issue_num(self) -> u16 { - match self { - Self::Diagnostic => 5320, - } + match self {} + } +} + +impl crate::diagnostic_filter::Severity { + #[cfg(feature = "wgsl-in")] + pub(crate) fn report_wgsl_parse_diag<'a>( + self, + err: crate::front::wgsl::error::Error<'a>, + source: &str, + ) -> Result<(), crate::front::wgsl::error::Error<'a>> { + self.report_diag(err, |e, level| { + let e = e.as_parse_error(source); + log::log!(level, "{}", e.emit_to_string(source)); + }) } } @@ -75,25 +86,12 @@ mod test { use super::{DirectiveKind, UnimplementedDirectiveKind}; #[test] + #[allow(clippy::never_loop, unreachable_code, unused_variables)] fn unimplemented_directives() { for unsupported_shader in UnimplementedDirectiveKind::iter() { let shader; let expected_msg; - match unsupported_shader { - UnimplementedDirectiveKind::Diagnostic => { - shader = "diagnostic(off,derivative_uniformity);"; - expected_msg = "\ -error: the `diagnostic` directive is not yet implemented - ┌─ wgsl:1:1 - │ -1 │ diagnostic(off,derivative_uniformity); - │ ^^^^^^^^^^ this global directive is standard, but not yet implemented - │ - = note: Let Naga maintainers know that you ran into this at , so they can prioritize it! - -"; - } - }; + match unsupported_shader {}; assert_parse_err(shader, expected_msg); } @@ -105,7 +103,7 @@ error: the `diagnostic` directive is not yet implemented let directive; let expected_msg; match unsupported_shader { - DirectiveKind::Unimplemented(UnimplementedDirectiveKind::Diagnostic) => { + DirectiveKind::Diagnostic => { directive = "diagnostic(off,derivative_uniformity)"; expected_msg = "\ error: expected global declaration, but found a global directive @@ -144,6 +142,7 @@ error: expected global declaration, but found a global directive "; } + DirectiveKind::Unimplemented(kind) => match kind {}, } let shader = format!( diff --git a/naga/src/front/wgsl/parse/mod.rs b/naga/src/front/wgsl/parse/mod.rs index fcfcc3775..379a8f2b8 100644 --- a/naga/src/front/wgsl/parse/mod.rs +++ b/naga/src/front/wgsl/parse/mod.rs @@ -1,3 +1,4 @@ +use crate::diagnostic_filter::{self, DiagnosticFilter, FilterableTriggeringRule}; use crate::front::wgsl::error::{Error, ExpectedToken}; use crate::front::wgsl::parse::directive::enable_extension::{ EnableExtension, EnableExtensions, UnimplementedEnableExtension, @@ -2529,6 +2530,17 @@ impl Parser { self.push_rule_span(Rule::Directive, &mut lexer); let _ = lexer.next_ident_with_span().unwrap(); match kind { + DirectiveKind::Diagnostic => { + if let Some(diagnostic_filter) = self.diagnostic_filter(&mut lexer)? { + let triggering_rule = diagnostic_filter.triggering_rule; + let span = self.peek_rule_span(&lexer); + Err(Error::DiagnosticNotYetImplemented { + triggering_rule, + span, + })?; + } + lexer.expect(Token::Separator(';'))?; + } DirectiveKind::Enable => { self.directive_ident_list(&mut lexer, |ident, span| { let kind = EnableExtension::from_ident(ident, span)?; @@ -2614,4 +2626,55 @@ impl Parser { } Ok(brace_nesting_level + 1) } + + fn diagnostic_filter<'a>( + &self, + lexer: &mut Lexer<'a>, + ) -> Result, Error<'a>> { + lexer.expect(Token::Paren('('))?; + + let (severity_control_name, severity_control_name_span) = lexer.next_ident_with_span()?; + let new_severity = diagnostic_filter::Severity::from_ident(severity_control_name).ok_or( + Error::DiagnosticInvalidSeverity { + severity_control_name_span, + }, + )?; + + lexer.expect(Token::Separator(','))?; + + let (diagnostic_name_token, diagnostic_name_token_span) = lexer.next_ident_with_span()?; + let diagnostic_rule_name = if lexer.skip(Token::Separator('.')) { + // Don't try to validate these name tokens on two tokens, which is conventionally used + // for third-party tooling. + lexer.next_ident_with_span()?; + None + } else { + Some(diagnostic_name_token) + }; + let diagnostic_rule_name_span = diagnostic_name_token_span; + + let filter = diagnostic_rule_name + .and_then(|name| { + FilterableTriggeringRule::from_ident(name) + .map(Ok) + .or_else(|| { + diagnostic_filter::Severity::Warning + .report_wgsl_parse_diag( + Error::UnknownDiagnosticRuleName(diagnostic_rule_name_span), + lexer.source, + ) + .err() + .map(Err) + }) + }) + .transpose()? + .map(|triggering_rule| DiagnosticFilter { + new_severity, + triggering_rule, + }); + lexer.skip(Token::Separator(',')); + lexer.expect(Token::Paren(')'))?; + + Ok(filter) + } } diff --git a/naga/src/lib.rs b/naga/src/lib.rs index 145b95f66..912ca4f42 100644 --- a/naga/src/lib.rs +++ b/naga/src/lib.rs @@ -255,6 +255,7 @@ pub mod back; mod block; #[cfg(feature = "compact")] pub mod compact; +pub mod diagnostic_filter; pub mod error; pub mod front; pub mod keywords;