Skip to content

Commit

Permalink
WIP: 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 23, 2024
1 parent 334d36d commit 2859ecf
Show file tree
Hide file tree
Showing 5 changed files with 280 additions and 26 deletions.
125 changes: 125 additions & 0 deletions naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
//! Functionality for managing [`DiagnosticFilter`]s.
//!
//! This functionality is typically used by front ends via [`DiagnosticFilterMap`].

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 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 occurence's span.

Check warning on line 70 in naga/src/diagnostic_filter.rs

View workflow job for this annotation

GitHub Actions / Format & Typos

"occurence" should be "occurrence".
///
/// 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>>,
}
76 changes: 76 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,58 @@ 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 {
// TODO: This should be a warning, not an error!
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(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
Loading

0 comments on commit 2859ecf

Please sign in to comment.