Skip to content

Commit

Permalink
feat(wgsl-in): parse diagnostic(…) directives (with unimpl. err.)
Browse files Browse the repository at this point in the history
  • Loading branch information
ErichDonGubler committed Oct 30, 2024
1 parent 6934f5a commit 8c13d8f
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 27 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
97 changes: 97 additions & 0 deletions naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//! [`DiagnosticFilter`]s and supporting functionality.
/// A severity set on a [`DiagnosticFilter`].
///
/// <https://www.w3.org/TR/WGSL/#diagnostic-severity>
#[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<Self> {
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<E>(
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`].
///
/// <https://www.w3.org/TR/WGSL/#filterable-triggering-rules>
#[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<Self> {
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.
///
/// <https://www.w3.org/TR/WGSL/#diagnostic-filter>
#[derive(Clone, Debug)]
pub struct DiagnosticFilter {
pub new_severity: Severity,
pub triggering_rule: FilterableTriggeringRule,
}
50 changes: 50 additions & 0 deletions naga/src/front/wgsl/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::diagnostic_filter::FilterableTriggeringRule;
use crate::front::wgsl::parse::directive::enable_extension::{
EnableExtension, UnimplementedEnableExtension,
};
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 ",
"<https://www.w3.org/TR/WGSL/#filterable-triggering-rules>."
)
.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())],
Expand Down Expand Up @@ -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 ",
"<https://www.w3.org/TR/WGSL/#diagnostic-severity>."
)
.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 ",
"<https://github.com/gfx-rs/wgpu/issues/{}>, ",
"so they can prioritize it!"
),
triggering_rule.tracking_issue_num()
)],
},
}
}
}
Expand Down
53 changes: 26 additions & 27 deletions naga/src/front/wgsl/parse/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand All @@ -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<Self> {
Some(match s {
Self::DIAGNOSTIC => Self::Unimplemented(UnimplementedDirectiveKind::Diagnostic),
Self::DIAGNOSTIC => Self::Diagnostic,
Self::ENABLE => Self::Enable,
Self::REQUIRES => Self::Requires,
_ => return None,
Expand All @@ -33,19 +35,18 @@ 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 {},
}
}

#[cfg(test)]
fn iter() -> impl Iterator<Item = Self> {
use strum::IntoEnumIterator;

[Self::Enable, Self::Requires]
[Self::Diagnostic, Self::Enable, Self::Requires]
.into_iter()
.chain(UnimplementedDirectiveKind::iter().map(Self::Unimplemented))
}
Expand All @@ -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));
})
}
}

Expand All @@ -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 <https://github.com/gfx-rs/wgpu/issues/5320>, so they can prioritize it!
";
}
};
match unsupported_shader {};

assert_parse_err(shader, expected_msg);
}
Expand All @@ -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
Expand Down Expand Up @@ -144,6 +142,7 @@ error: expected global declaration, but found a global directive
";
}
DirectiveKind::Unimplemented(kind) => match kind {},
}

let shader = format!(
Expand Down
63 changes: 63 additions & 0 deletions naga/src/front/wgsl/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -2614,4 +2626,55 @@ impl Parser {
}
Ok(brace_nesting_level + 1)
}

fn diagnostic_filter<'a>(
&self,
lexer: &mut Lexer<'a>,
) -> Result<Option<DiagnosticFilter>, 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)
}
}
1 change: 1 addition & 0 deletions naga/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 8c13d8f

Please sign in to comment.