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

WGSL: add base support for requires that reports nice errors #6437

Merged
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
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 an internal skeleton for parsing `requires`, `enable`, and `diagnostic` directives that don't yet do anything besides emit nicer errors. By @ErichDonGubler in [#6352](https://github.com/gfx-rs/wgpu/pull/6352), [#6424](https://github.com/gfx-rs/wgpu/pull/6424).
- 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).
- 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
32 changes: 32 additions & 0 deletions naga/src/front/wgsl/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::front::wgsl::parse::directive::enable_extension::{
EnableExtension, UnimplementedEnableExtension,
};
use crate::front::wgsl::parse::directive::language_extension::{
LanguageExtension, UnimplementedLanguageExtension,
};
use crate::front::wgsl::parse::directive::{DirectiveKind, UnimplementedDirectiveKind};
use crate::front::wgsl::parse::lexer::Token;
use crate::front::wgsl::Scalar;
Expand Down Expand Up @@ -190,6 +193,7 @@ pub(crate) enum Error<'a> {
UnknownStorageFormat(Span),
UnknownConservativeDepth(Span),
UnknownEnableExtension(Span, &'a str),
UnknownLanguageExtension(Span, &'a str),
SizeAttributeTooLow(Span, u32),
AlignAttributeTooLow(Span, Alignment),
NonPowerOfTwoAlignAttribute(Span),
Expand Down Expand Up @@ -287,6 +291,10 @@ pub(crate) enum Error<'a> {
kind: EnableExtension,
span: Span,
},
LanguageExtensionNotYetImplemented {
kind: UnimplementedLanguageExtension,
span: Span,
},
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -545,6 +553,15 @@ impl<'a> Error<'a> {
.into(),
],
},
Error::UnknownLanguageExtension(span, name) => ParseError {
message: format!("unknown language extension `{name}`"),
labels: vec![(span, "".into())],
notes: vec![concat!(
"See available extensions at ",
"<https://www.w3.org/TR/WGSL/#language-extensions-sec>."
)
.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 @@ -976,6 +993,21 @@ impl<'a> Error<'a> {
vec![]
},
},
Error::LanguageExtensionNotYetImplemented { kind, span } => ParseError {
message: format!(
"the `{}` language extension is not yet supported",
LanguageExtension::Unimplemented(kind).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!"
),
kind.tracking_issue_num()
)],
},
}
}
}
Expand Down
24 changes: 6 additions & 18 deletions naga/src/front/wgsl/parse/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@
//! See also <https://www.w3.org/TR/WGSL/#directives>.

pub mod enable_extension;
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 enum DirectiveKind {
/// An [`enable_extension`].
Enable,
/// A language extension.
Requires,
Unimplemented(UnimplementedDirectiveKind),
}

Expand All @@ -22,7 +25,7 @@ impl DirectiveKind {
Some(match s {
Self::DIAGNOSTIC => Self::Unimplemented(UnimplementedDirectiveKind::Diagnostic),
Self::ENABLE => Self::Enable,
Self::REQUIRES => Self::Unimplemented(UnimplementedDirectiveKind::Requires),
Self::REQUIRES => Self::Requires,
_ => return None,
})
}
Expand All @@ -31,9 +34,9 @@ impl DirectiveKind {
pub const fn to_ident(self) -> &'static str {
match self {
Self::Enable => Self::ENABLE,
Self::Requires => Self::REQUIRES,
Self::Unimplemented(kind) => match kind {
UnimplementedDirectiveKind::Diagnostic => Self::DIAGNOSTIC,
UnimplementedDirectiveKind::Requires => Self::REQUIRES,
},
}
}
Expand All @@ -51,14 +54,12 @@ impl DirectiveKind {
#[cfg_attr(test, derive(strum::EnumIter))]
pub enum UnimplementedDirectiveKind {
Diagnostic,
Requires,
}

impl UnimplementedDirectiveKind {
pub const fn tracking_issue_num(self) -> u16 {
match self {
Self::Diagnostic => 5320,
Self::Requires => 6350,
}
}
}
Expand Down Expand Up @@ -88,19 +89,6 @@ error: `diagnostic` is 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!

";
}
UnimplementedDirectiveKind::Requires => {
shader = "requires readonly_and_readwrite_storage_textures";
expected_msg = "\
error: `requires` is not yet implemented
┌─ wgsl:1:1
1 │ requires readonly_and_readwrite_storage_textures
│ ^^^^^^^^ 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/6350>, so they can prioritize it!

";
}
};
Expand Down Expand Up @@ -141,7 +129,7 @@ error: expected global declaration, but found a global directive

";
}
DirectiveKind::Unimplemented(UnimplementedDirectiveKind::Requires) => {
DirectiveKind::Requires => {
directive = "requires readonly_and_readwrite_storage_textures";
expected_msg = "\
error: expected global declaration, but found a global directive
Expand Down
85 changes: 85 additions & 0 deletions naga/src/front/wgsl/parse/directive/language_extension.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//! `requires …;` extensions in WGSL.
//!
//! The focal point of this module is the [`LanguageExtension`] API.

/// A language extension not guaranteed to be present in all environments.
///
/// WGSL spec.: <https://www.w3.org/TR/WGSL/#language-extensions-sec>
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub(crate) enum LanguageExtension {
#[allow(unused)]
Implemented(ImplementedLanguageExtension),
Unimplemented(UnimplementedLanguageExtension),
}

impl LanguageExtension {
const READONLY_AND_READWRITE_STORAGE_TEXTURES: &'static str =
"readonly_and_readwrite_storage_textures";
const PACKED4X8_INTEGER_DOT_PRODUCT: &'static str = "packed_4x8_integer_dot_product";
const UNRESTRICTED_POINTER_PARAMETERS: &'static str = "unrestricted_pointer_parameters";
const POINTER_COMPOSITE_ACCESS: &'static str = "pointer_composite_access";

/// Convert from a sentinel word in WGSL into its associated [`LanguageExtension`], if possible.
pub fn from_ident(s: &str) -> Option<Self> {
Some(match s {
Self::READONLY_AND_READWRITE_STORAGE_TEXTURES => Self::Unimplemented(
UnimplementedLanguageExtension::ReadOnlyAndReadWriteStorageTextures,
),
Self::PACKED4X8_INTEGER_DOT_PRODUCT => {
Self::Unimplemented(UnimplementedLanguageExtension::Packed4x8IntegerDotProduct)
}
Self::UNRESTRICTED_POINTER_PARAMETERS => {
Self::Unimplemented(UnimplementedLanguageExtension::UnrestrictedPointerParameters)
}
Self::POINTER_COMPOSITE_ACCESS => {
Self::Unimplemented(UnimplementedLanguageExtension::PointerCompositeAccess)
}
_ => return None,
})
}

/// Maps this [`LanguageExtension`] into the sentinel word associated with it in WGSL.
pub const fn to_ident(self) -> &'static str {
match self {
Self::Implemented(kind) => match kind {},
Self::Unimplemented(kind) => match kind {
UnimplementedLanguageExtension::ReadOnlyAndReadWriteStorageTextures => {
Self::READONLY_AND_READWRITE_STORAGE_TEXTURES
}
UnimplementedLanguageExtension::Packed4x8IntegerDotProduct => {
Self::PACKED4X8_INTEGER_DOT_PRODUCT
}
UnimplementedLanguageExtension::UnrestrictedPointerParameters => {
Self::UNRESTRICTED_POINTER_PARAMETERS
}
UnimplementedLanguageExtension::PointerCompositeAccess => {
Self::POINTER_COMPOSITE_ACCESS
}
},
}
}
}

/// A variant of [`LanguageExtension::Implemented`].
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub(crate) enum ImplementedLanguageExtension {}

/// A variant of [`LanguageExtension::Unimplemented`].
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub(crate) enum UnimplementedLanguageExtension {
ReadOnlyAndReadWriteStorageTextures,
Packed4x8IntegerDotProduct,
UnrestrictedPointerParameters,
PointerCompositeAccess,
}

impl UnimplementedLanguageExtension {
pub(crate) const fn tracking_issue_num(self) -> u16 {
match self {
Self::ReadOnlyAndReadWriteStorageTextures => 6204,
Self::Packed4x8IntegerDotProduct => 6445,
Self::UnrestrictedPointerParameters => 5158,
Self::PointerCompositeAccess => 6192,
}
}
}
18 changes: 18 additions & 0 deletions naga/src/front/wgsl/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::front::wgsl::error::{Error, ExpectedToken};
use crate::front::wgsl::parse::directive::enable_extension::{
EnableExtension, EnableExtensions, UnimplementedEnableExtension,
};
use crate::front::wgsl::parse::directive::language_extension::LanguageExtension;
use crate::front::wgsl::parse::directive::DirectiveKind;
use crate::front::wgsl::parse::lexer::{Lexer, Token};
use crate::front::wgsl::parse::number::Number;
Expand Down Expand Up @@ -2544,6 +2545,23 @@ impl Parser {
Ok(())
})?;
}
DirectiveKind::Requires => {
self.directive_ident_list(&mut lexer, |ident, span| {
match LanguageExtension::from_ident(ident) {
Some(LanguageExtension::Implemented(_kind)) => {
// NOTE: No further validation is needed for an extension, so
// just throw parsed information away. If we ever want to apply
// what we've parsed to diagnostics, maybe we'll want to refer
// to enabled extensions later?
Ok(())
}
Some(LanguageExtension::Unimplemented(kind)) => {
Err(Error::LanguageExtensionNotYetImplemented { kind, span })
}
None => Err(Error::UnknownLanguageExtension(span, ident)),
}
})?;
}
DirectiveKind::Unimplemented(kind) => {
return Err(Error::DirectiveNotYetImplemented { kind, span })
}
Expand Down