Skip to content

Commit

Permalink
feat(wgsl-in): parse diagnostic(…) directives (with no effect)
Browse files Browse the repository at this point in the history
  • Loading branch information
ErichDonGubler committed Oct 24, 2024
1 parent 334d36d commit 555c643
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 27 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216).
- Support local `const` declarations in WGSL. By @sagudev in [#6156](https://github.com/gfx-rs/wgpu/pull/6156).
- Implemented `const_assert` in WGSL. By @sagudev in [#6198](https://github.com/gfx-rs/wgpu/pull/6198).
- Support polyfilling `inverse` in WGSL. By @chyyran in [#6385](https://github.com/gfx-rs/wgpu/pull/6385).
- Add base support for parsing `requires`, `enable`, and `diagnostic` directives. No extensions or diagnostic filters are yet supported, but diagnostics have improved dramatically. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437).
- Add base support for parsing `requires`, `enable`, and `diagnostic` directives. No extensions or diagnostic filters are yet supported, but diagnostics have improved dramatically. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424), [#6437](https://github.com/gfx-rs/wgpu/pull/6437), [#6456](https://github.com/gfx-rs/wgpu/pull/6456).
- Include error chain information as a message and notes in shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436).
- Unify Naga CLI error output with the format of shader compilation messages. By @ErichDonGubler in [#6436](https://github.com/gfx-rs/wgpu/pull/6436).

Expand Down
123 changes: 123 additions & 0 deletions naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
//! [`DiagnosticFilter`]s and supporting functionality.

use crate::{Handle, Span};

use indexmap::IndexMap;

/// 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<Self> {
Some(match s {
Self::ERROR => Self::Error,
Self::WARNING => Self::Warning,
Self::INFO => Self::Info,
Self::OFF => Self::Off,
_ => return None,
})
}
}

/// The rule being configured in a [`DiagnosticFilter`].
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub enum DiagnosticTriggeringRule {
DerivativeUniformity,
}

impl DiagnosticTriggeringRule {
const DERIVATIVE_UNIFORMITY: &'static str = "derivative_uniformity";

/// Convert from a sentinel word in WGSL into its associated [`DiagnosticTriggeringRule`], if possible.
pub fn from_ident(s: &str) -> Option<Self> {
Some(match s {
Self::DERIVATIVE_UNIFORMITY => Self::DerivativeUniformity,
_ => return None,
})
}

/// Maps this [`DiagnosticTriggeringRule`] into the sentinel word associated with it in WGSL.
pub const fn to_ident(self) -> &'static str {
match self {
Self::DerivativeUniformity => Self::DERIVATIVE_UNIFORMITY,
}
}
}

/// 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: DiagnosticTriggeringRule,
}

/// 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)]
pub(crate) struct DiagnosticFilterMap(IndexMap<DiagnosticTriggeringRule, (Severity, Span)>);

#[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));
Ok(())
}
Entry::Occupied(entry) => {
return Err(ConflictingDiagnosticRuleError {
triggering_rule,
triggering_rule_spans: [entry.get().1, span],
})
}
}
}
}

/// An error yielded by [`DiagnosticFilterMap::add`].
#[cfg(feature = "wgsl-in")]
#[derive(Clone, Debug)]
pub(crate) struct ConflictingDiagnosticRuleError {
pub triggering_rule: DiagnosticTriggeringRule,
pub triggering_rule_spans: [Span; 2],
}

/// Represents a single link in a linked list of [`DiagnosticFilter`]s backed by a
/// [`crate::Arena`].
#[derive(Clone, Debug)]
pub struct DiagnosticFilterNode {
pub inner: DiagnosticFilter,
pub parent: Option<Handle<DiagnosticFilterNode>>,
}
75 changes: 75 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::{ConflictingDiagnosticRuleError, DiagnosticTriggeringRule};
use crate::front::wgsl::parse::directive::enable_extension::{
EnableExtension, UnimplementedEnableExtension,
};
Expand Down Expand Up @@ -295,6 +296,29 @@ pub(crate) enum Error<'a> {
kind: UnimplementedLanguageExtension,
span: Span,
},
DiagnosticInvalidSeverity {
severity_control_name_span: Span,
},
DiagnosticInvalidRuleName {
diagnostic_rule_name_span: Span,
},
DiagnosticDuplicateTriggeringRule {
triggering_rule: DiagnosticTriggeringRule,
triggering_rule_spans: [Span; 2],
},
}

impl<'a> From<ConflictingDiagnosticRuleError> for Error<'a> {
fn from(value: ConflictingDiagnosticRuleError) -> Self {
let ConflictingDiagnosticRuleError {
triggering_rule,
triggering_rule_spans,
} = value;
Self::DiagnosticDuplicateTriggeringRule {
triggering_rule,
triggering_rule_spans,
}
}
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -1008,6 +1032,57 @@ 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::DiagnosticInvalidRuleName {
diagnostic_rule_name_span,
} => ParseError {
message: "invalid `diagnostic(…)` rule name".into(),
labels: vec![(
diagnostic_rule_name_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::DiagnosticDuplicateTriggeringRule {
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()],
}
}
}
}
}
Expand Down
37 changes: 11 additions & 26 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,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 {},
}
}

Expand All @@ -52,15 +53,11 @@ 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 {}
}
}

Expand All @@ -73,25 +70,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 @@ -103,7 +87,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 @@ -142,6 +126,7 @@ error: expected global declaration, but found a global directive
";
}
DirectiveKind::Unimplemented(kind) => match kind {},
}

let shader = format!(
Expand Down
54 changes: 54 additions & 0 deletions naga/src/front/wgsl/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
use crate::diagnostic_filter::{
DiagnosticFilter, DiagnosticFilterMap, DiagnosticTriggeringRule, Severity,
};
use crate::front::wgsl::error::{Error, ExpectedToken};
use crate::front::wgsl::parse::directive::enable_extension::{
EnableExtension, EnableExtensions, UnimplementedEnableExtension,
Expand Down Expand Up @@ -2520,13 +2523,21 @@ 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, span)) = lexer.peek_ident_with_span() {
if let Some(kind) = DirectiveKind::from_ident(ident) {
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 span = self.peek_rule_span(&lexer);
diagnostic_filters.add(diagnostic_filter, 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 @@ -2612,4 +2623,47 @@ 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 = 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
.map(|name| {
DiagnosticTriggeringRule::from_ident(name).ok_or(Error::DiagnosticInvalidRuleName {
diagnostic_rule_name_span,
})
})
.transpose()?
.map(|triggering_rule| DiagnosticFilter {
new_severity,
triggering_rule,
});
lexer.skip(Token::Separator(','));
lexer.expect(Token::Paren(')'))?;

Ok(filter)
}
}
Loading

0 comments on commit 555c643

Please sign in to comment.