Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse diagnostic(…) attributes on fns #6353

Draft
wants to merge 12 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
use std::str::FromStr;

use crate::{Handle, Span};

#[cfg(feature = "arbitrary")]
use arbitrary::Arbitrary;
use indexmap::IndexMap;
#[cfg(feature = "deserialize")]
use serde::Deserialize;
#[cfg(feature = "serialize")]
use serde::Serialize;

// TODO: docs
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub enum Severity {
Off,
Info,
Warning,
Error,
}

impl Severity {
pub fn from_ident(s: &str) -> Option<Self> {
Some(match s {
"error" => Self::Error,
"warning" => Self::Warning,
"info" => Self::Info,
"off" => Self::Off,
_ => return None,
})
}
}

impl FromStr for Severity {
type Err = ();

fn from_str(s: &str) -> Result<Self, Self::Err> {
Self::from_ident(s).ok_or(())
}
}

// TODO: docs
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub enum DiagnosticTriggeringRule {
DerivativeUniformity,
}

// TODO: docs
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub struct DiagnosticFilter {
pub new_severity: Severity,
pub triggering_rule: DiagnosticTriggeringRule,
}

// TODO: docs
#[derive(Clone, Debug, Default)]
// TODO: `FastIndexMap` mebbe?
pub(crate) struct DiagnosticFilterMap(IndexMap<DiagnosticTriggeringRule, (Severity, Span)>);

#[cfg(feature = "wgsl-in")]
impl DiagnosticFilterMap {
pub(crate) fn new() -> Self {
Self::default()
}

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],
})
}
}
}
}

#[cfg(feature = "wgsl-in")]
impl IntoIterator for DiagnosticFilterMap {
type Item = (DiagnosticTriggeringRule, (Severity, Span));

type IntoIter = indexmap::map::IntoIter<DiagnosticTriggeringRule, (Severity, Span)>;

fn into_iter(self) -> Self::IntoIter {
let Self(this) = self;
this.into_iter()
}
}

#[cfg(feature = "wgsl-in")]
#[derive(Clone, Debug)]
pub(crate) struct ConflictingDiagnosticRuleError {
pub triggering_rule: DiagnosticTriggeringRule,
pub triggering_rule_spans: [Span; 2],
}

// TODO: docs
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub struct DiagnosticFilterNode {
pub inner: DiagnosticFilter,
pub parent: Option<Handle<DiagnosticFilterNode>>,
}
113 changes: 113 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::lexer::Token;
use crate::front::wgsl::Scalar;
use crate::proc::{Alignment, ConstantEvaluatorError, ResolveError};
Expand Down Expand Up @@ -265,6 +266,38 @@ pub(crate) enum Error<'a> {
PipelineConstantIDValue(Span),
NotBool(Span),
ConstAssertFailed(Span),
DirectiveNotYetImplemented {
kind: &'static str,
span: Span,
tracking_issue_num: u16,
},
DirectiveAfterFirstGlobalDecl {
directive_span: Span,
},
DiagnosticInvalidSeverity {
severity_control_name_span: Span,
},
DiagnosticInvalidRuleName {
diagnostic_rule_name_span: Span,
},
DiagnosticDuplicateTriggeringRule {
triggering_rule: DiagnosticTriggeringRule,
triggering_rule_spans: [Span; 2],
},
}

// TODO: better place
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 @@ -861,6 +894,86 @@ impl<'a> Error<'a> {
labels: vec![(span, "evaluates to false".into())],
notes: vec![],
},
Error::DirectiveNotYetImplemented {
kind,
span,
tracking_issue_num,
} => ParseError {
message: format!("`{kind}` global directives are not yet supported (sorry!)"),
labels: vec![(
span,
concat!(
"this directive specifies standard functionality ",
"which is not yet implemented in Naga"
)
.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!"
),
tracking_issue_num
)],
},
Error::DirectiveAfterFirstGlobalDecl { directive_span } => ParseError {
message: "expected global declaration, but found a global directive".into(),
labels: vec![(
directive_span,
"this directive was written after global declaration parsing started".into(),
)],
notes: vec![concat!(
"global directives are only allowed before global declarations; ",
"maybe hoist this closer to the top of the shader?"
)
.into()],
},
Error::DiagnosticInvalidSeverity {
severity_control_name_span,
} => ParseError {
message: "invalid identifier in `diagnostic(…)` rule severity".into(),
labels: vec![(
severity_control_name_span,
"not a valid severity level".into(),
)],
// TODO: note expected values
notes: vec![],
},
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 rule name".into())],
// TODO: note expected values
notes: vec![],
},
Error::DiagnosticDuplicateTriggeringRule {
triggering_rule,
triggering_rule_spans,
} => {
let [first_span, second_span] = triggering_rule_spans;
ParseError {
message: format!(
"found conflicting `@diagnostic(…)` rules for {triggering_rule:?}"
),
// TODO: consider carrying spans for the rule name, too
labels: vec![
(first_span, "first rule's severity".into()),
(
second_span,
"second rule's severity, which conflicts".into(),
),
],
notes: vec![concat!(
"`@diagnostic(…)` rules conflict unless the severity is the same; ",
"delete a rule, or ",
"ensure that all severities with the same rule name match"
)
.into()],
}
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions naga/src/front/wgsl/lower/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
.arguments
.iter()
.enumerate()
.map(|(i, arg)| {
.map(|(i, arg)| -> Result<_, Error<'_>> {
let ty = self.resolve_ast_type(arg.ty, ctx)?;
let expr = expressions
.append(crate::Expression::FunctionArgument(i as u32), arg.name.span);
Expand All @@ -1263,7 +1263,7 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
let result = f
.result
.as_ref()
.map(|res| {
.map(|res| -> Result<_, Error<'_>> {
let ty = self.resolve_ast_type(res.ty, ctx)?;
Ok(crate::FunctionResult {
ty,
Expand Down
9 changes: 9 additions & 0 deletions naga/src/front/wgsl/parse/ast.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::diagnostic_filter::DiagnosticFilterNode;
use crate::front::wgsl::parse::number::Number;
use crate::front::wgsl::Scalar;
use crate::{Arena, FastIndexSet, Handle, Span};
Expand All @@ -24,6 +25,13 @@ pub struct TranslationUnit<'a> {
/// These are referred to by `Handle<ast::Type<'a>>` values.
/// User-defined types are referred to by name until lowering.
pub types: Arena<Type<'a>>,

/// Arena for all diagnostic filter rules parsed in this module, including those in functions.
pub diagnostic_filters: Arena<DiagnosticFilterNode>,
/// The head of a linked list of `diagnostic(…)` directives parsed in this module.
///
/// TODO: doc more
pub diagnostic_filter_head: Option<Handle<DiagnosticFilterNode>>,
}

#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -119,6 +127,7 @@ pub struct Function<'a> {
pub arguments: Vec<FunctionArgument<'a>>,
pub result: Option<FunctionResult<'a>>,
pub body: Block<'a>,
pub diagnostic_filter_head: Option<Handle<DiagnosticFilterNode>>,
}

#[derive(Debug)]
Expand Down
37 changes: 37 additions & 0 deletions naga/src/front/wgsl/parse/directive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
pub enum DirectiveKind {
Unimplemented(UnimplementedDirectiveKind),
Diagnostic,
}

impl DirectiveKind {
pub fn from_ident(s: &str) -> Option<(Self, &'static str)> {
Some(match s {
"diagnostic" => (Self::Diagnostic, "diagnostic"),
"enable" => (
Self::Unimplemented(UnimplementedDirectiveKind::Enable),
"enable",
),
"requires" => (
Self::Unimplemented(UnimplementedDirectiveKind::Requires),
"requires",
),
_ => return None,
})
}
}

#[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
pub enum UnimplementedDirectiveKind {
Enable,
Requires,
}

impl UnimplementedDirectiveKind {
pub const fn tracking_issue_num(self) -> u16 {
match self {
Self::Requires => 6350,
Self::Enable => 5476,
}
}
}
Loading