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

don't panic if naga parsing of shader source fails #5034

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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 4 additions & 19 deletions naga-cli/src/bin/naga.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,10 @@ fn parse_input(
},
&input,
)
.unwrap_or_else(|errors| {
let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str);
emit_glsl_parser_error(errors, filename.unwrap_or("glsl"), &input);
.unwrap_or_else(|error| {
let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str).unwrap_or("glsl");
let mut writer = StandardStream::stderr(ColorChoice::Auto);
error.emit_to_writer_with_path(&mut writer, &input, filename);
std::process::exit(1);
}),
Some(input),
Expand Down Expand Up @@ -719,22 +720,6 @@ use codespan_reporting::{
};
use naga::WithSpan;

pub fn emit_glsl_parser_error(errors: Vec<naga::front::glsl::Error>, filename: &str, source: &str) {
let files = SimpleFile::new(filename, source);
let config = codespan_reporting::term::Config::default();
let writer = StandardStream::stderr(ColorChoice::Auto);

for err in errors {
let mut diagnostic = Diagnostic::error().with_message(err.kind.to_string());

if let Some(range) = err.meta.to_range() {
diagnostic = diagnostic.with_labels(vec![Label::primary((), range)]);
}

term::emit(&mut writer.lock(), &config, &files, &diagnostic).expect("cannot write error");
}
}

pub fn emit_annotated_error<E: Error>(ann_err: &WithSpan<E>, filename: &str, source: &str) {
let files = SimpleFile::new(filename, source);
let config = codespan_reporting::term::Config::default();
Expand Down
63 changes: 60 additions & 3 deletions naga/src/front/glsl/error.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
use super::token::TokenValue;
use crate::{proc::ConstantEvaluatorError, Span};
use codespan_reporting::diagnostic::{Diagnostic, Label};
use codespan_reporting::files::SimpleFile;
use codespan_reporting::term;
use pp_rs::token::PreprocessorError;
use std::borrow::Cow;
use termcolor::{NoColor, WriteColor};
use thiserror::Error;

fn join_with_comma(list: &[ExpectedToken]) -> String {
Expand All @@ -18,7 +22,7 @@ fn join_with_comma(list: &[ExpectedToken]) -> String {
}

/// One of the expected tokens returned in [`InvalidToken`](ErrorKind::InvalidToken).
#[derive(Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum ExpectedToken {
/// A specific token was expected.
Token(TokenValue),
Expand Down Expand Up @@ -55,7 +59,7 @@ impl std::fmt::Display for ExpectedToken {
}

/// Information about the cause of an error.
#[derive(Debug, Error)]
#[derive(Clone, Debug, Error)]
#[cfg_attr(test, derive(PartialEq))]
pub enum ErrorKind {
/// Whilst parsing as encountered an unexpected EOF.
Expand Down Expand Up @@ -123,7 +127,7 @@ impl From<ConstantEvaluatorError> for ErrorKind {
}

/// Error returned during shader parsing.
#[derive(Debug, Error)]
#[derive(Clone, Debug, Error)]
#[error("{kind}")]
#[cfg_attr(test, derive(PartialEq))]
pub struct Error {
Expand All @@ -132,3 +136,56 @@ pub struct Error {
/// Holds information about the range of the source code where the error happened.
pub meta: Span,
}

/// A collection of errors returned during shader parsing.
#[derive(Clone, Debug)]
#[cfg_attr(test, derive(PartialEq))]
pub struct ParseError {
pub errors: Vec<Error>,
}

impl ParseError {
pub fn emit_to_writer(&self, writer: &mut impl WriteColor, source: &str) {
self.emit_to_writer_with_path(writer, source, "glsl");
}

pub fn emit_to_writer_with_path(&self, writer: &mut impl WriteColor, source: &str, path: &str) {
let path = path.to_string();
let files = SimpleFile::new(path, source);
let config = codespan_reporting::term::Config::default();

for err in &self.errors {
let mut diagnostic = Diagnostic::error().with_message(err.kind.to_string());

if let Some(range) = err.meta.to_range() {
diagnostic = diagnostic.with_labels(vec![Label::primary((), range)]);
}

term::emit(writer, &config, &files, &diagnostic).expect("cannot write error");
}
}

pub fn emit_to_string(&self, source: &str) -> String {
let mut writer = NoColor::new(Vec::new());
self.emit_to_writer(&mut writer, source);
String::from_utf8(writer.into_inner()).unwrap()
}
}

impl std::fmt::Display for ParseError {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
self.errors.iter().try_for_each(|e| write!(f, "{e:?}"))
}
}

impl std::error::Error for ParseError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
}

impl From<Vec<Error>> for ParseError {
fn from(errors: Vec<Error>) -> Self {
Self { errors }
}
}
8 changes: 4 additions & 4 deletions naga/src/front/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ To begin, take a look at the documentation for the [`Frontend`].
*/

pub use ast::{Precision, Profile};
pub use error::{Error, ErrorKind, ExpectedToken};
pub use error::{Error, ErrorKind, ExpectedToken, ParseError};
pub use token::TokenValue;

use crate::{proc::Layouter, FastHashMap, FastHashSet, Handle, Module, ShaderStage, Span, Type};
Expand Down Expand Up @@ -196,7 +196,7 @@ impl Frontend {
&mut self,
options: &Options,
source: &str,
) -> std::result::Result<Module, Vec<Error>> {
) -> std::result::Result<Module, ParseError> {
self.reset(options.stage);

let lexer = lex::Lexer::new(source, &options.defines);
Expand All @@ -207,12 +207,12 @@ impl Frontend {
if self.errors.is_empty() {
Ok(module)
} else {
Err(std::mem::take(&mut self.errors))
Err(std::mem::take(&mut self.errors).into())
}
}
Err(e) => {
self.errors.push(e);
Err(std::mem::take(&mut self.errors))
Err(std::mem::take(&mut self.errors).into())
}
}
}
Expand Down
90 changes: 52 additions & 38 deletions naga/src/front/glsl/parser_tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{
ast::Profile,
error::ExpectedToken,
error::{Error, ErrorKind},
error::{Error, ErrorKind, ParseError},
token::TokenValue,
Frontend, Options, Span,
};
Expand All @@ -21,10 +21,12 @@ fn version() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::InvalidVersion(99000),
meta: Span::new(9, 14)
}],
ParseError {
errors: vec![Error {
kind: ErrorKind::InvalidVersion(99000),
meta: Span::new(9, 14)
}],
},
);

assert_eq!(
Expand All @@ -35,10 +37,12 @@ fn version() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::InvalidVersion(449),
meta: Span::new(9, 12)
}]
ParseError {
errors: vec![Error {
kind: ErrorKind::InvalidVersion(449),
meta: Span::new(9, 12)
}]
},
);

assert_eq!(
Expand All @@ -49,10 +53,12 @@ fn version() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::InvalidProfile("smart".into()),
meta: Span::new(13, 18),
}]
ParseError {
errors: vec![Error {
kind: ErrorKind::InvalidProfile("smart".into()),
meta: Span::new(13, 18),
}]
},
);

assert_eq!(
Expand All @@ -63,19 +69,21 @@ fn version() {
)
.err()
.unwrap(),
vec![
Error {
kind: ErrorKind::PreprocessorError(PreprocessorError::UnexpectedHash,),
meta: Span::new(27, 28),
},
Error {
kind: ErrorKind::InvalidToken(
TokenValue::Identifier("version".into()),
vec![ExpectedToken::Eof]
),
meta: Span::new(28, 35)
}
]
ParseError {
errors: vec![
Error {
kind: ErrorKind::PreprocessorError(PreprocessorError::UnexpectedHash,),
meta: Span::new(27, 28),
},
Error {
kind: ErrorKind::InvalidToken(
TokenValue::Identifier("version".into()),
vec![ExpectedToken::Eof]
),
meta: Span::new(28, 35)
}
]
},
);

// valid versions
Expand Down Expand Up @@ -447,10 +455,12 @@ fn functions() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::SemanticError("Function already defined".into()),
meta: Span::new(134, 152),
}]
ParseError {
errors: vec![Error {
kind: ErrorKind::SemanticError("Function already defined".into()),
meta: Span::new(134, 152),
}]
},
);

println!();
Expand Down Expand Up @@ -626,10 +636,12 @@ fn implicit_conversions() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::SemanticError("Unknown function \'test\'".into()),
meta: Span::new(156, 165),
}]
ParseError {
errors: vec![Error {
kind: ErrorKind::SemanticError("Unknown function \'test\'".into()),
meta: Span::new(156, 165),
}]
},
);

assert_eq!(
Expand All @@ -648,10 +660,12 @@ fn implicit_conversions() {
)
.err()
.unwrap(),
vec![Error {
kind: ErrorKind::SemanticError("Ambiguous best function for \'test\'".into()),
meta: Span::new(158, 165),
}]
ParseError {
errors: vec![Error {
kind: ErrorKind::SemanticError("Ambiguous best function for \'test\'".into()),
meta: Span::new(158, 165),
}]
}
);
}

Expand Down
2 changes: 1 addition & 1 deletion naga/src/front/glsl/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub struct Token {
///
/// This type is exported since it's returned in the
/// [`InvalidToken`](super::ErrorKind::InvalidToken) error.
#[derive(Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum TokenValue {
Identifier(String),

Expand Down
25 changes: 25 additions & 0 deletions naga/src/front/spv/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use super::ModuleState;
use crate::arena::Handle;
use codespan_reporting::diagnostic::Diagnostic;
use codespan_reporting::files::SimpleFile;
use codespan_reporting::term;
use termcolor::{NoColor, WriteColor};

#[derive(Debug, thiserror::Error)]
pub enum Error {
Expand Down Expand Up @@ -127,3 +131,24 @@ pub enum Error {
)]
NonBindingArrayOfImageOrSamplers,
}

impl Error {
pub fn emit_to_writer(&self, writer: &mut impl WriteColor, source: &str) {
self.emit_to_writer_with_path(writer, source, "glsl");
}

pub fn emit_to_writer_with_path(&self, writer: &mut impl WriteColor, source: &str, path: &str) {
let path = path.to_string();
let files = SimpleFile::new(path, source);
let config = codespan_reporting::term::Config::default();
let diagnostic = Diagnostic::error().with_message(format!("{self:?}"));

term::emit(writer, &config, &files, &diagnostic).expect("cannot write error");
}

pub fn emit_to_string(&self, source: &str) -> String {
let mut writer = NoColor::new(Vec::new());
self.emit_to_writer(&mut writer, source);
String::from_utf8(writer.into_inner()).unwrap()
}
}
6 changes: 2 additions & 4 deletions naga/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,7 @@ pub enum ImageClass {
}

/// A data type declared in the module.
#[derive(Debug, Eq, Hash, PartialEq)]
#[cfg_attr(feature = "clone", derive(Clone))]
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
Expand All @@ -700,8 +699,7 @@ pub struct Type {
}

/// Enum with additional information, depending on the kind of type.
#[derive(Debug, Eq, Hash, PartialEq)]
#[cfg_attr(feature = "clone", derive(Clone))]
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
Expand Down
Loading