From 3c0929247ff95013805138fd0269ac0a0a595199 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 1 Jun 2021 20:19:23 +0200 Subject: [PATCH 1/3] [glsl-in] include information on what was expected in `InvalidToken` --- src/front/glsl/error.rs | 50 ++++++++++++-- src/front/glsl/parser.rs | 119 ++++++++++++++++++++++++++++----- src/front/glsl/parser_tests.rs | 12 ++-- 3 files changed, 155 insertions(+), 26 deletions(-) diff --git a/src/front/glsl/error.rs b/src/front/glsl/error.rs index 924f411d0c..bb2baa972a 100644 --- a/src/front/glsl/error.rs +++ b/src/front/glsl/error.rs @@ -1,10 +1,52 @@ use super::{ constants::ConstantSolvingError, - token::{SourceMetadata, Token}, + token::{SourceMetadata, Token, TokenValue}, }; use std::borrow::Cow; use thiserror::Error; +fn join_with_comma(list: &[ExpectedToken]) -> String { + let mut string = "".to_string(); + for (i, val) in list.iter().enumerate() { + string.push_str(&val.to_string()); + match i { + i if i == list.len() - 1 => {} + i if i == list.len() - 2 => string.push_str(" or "), + _ => string.push_str(", "), + } + } + string +} + +#[derive(Debug, PartialEq)] +pub enum ExpectedToken { + Token(TokenValue), + TypeName, + Identifier, + IntLiteral, + FloatLiteral, + BoolLiteral, + EOF, +} +impl From for ExpectedToken { + fn from(token: TokenValue) -> Self { + ExpectedToken::Token(token) + } +} +impl std::fmt::Display for ExpectedToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match *self { + ExpectedToken::Token(ref token) => write!(f, "{:?}", token), + ExpectedToken::TypeName => write!(f, "a type"), + ExpectedToken::Identifier => write!(f, "identifier"), + ExpectedToken::IntLiteral => write!(f, "integer literal"), + ExpectedToken::FloatLiteral => write!(f, "float literal"), + ExpectedToken::BoolLiteral => write!(f, "bool literal"), + ExpectedToken::EOF => write!(f, "end of file"), + } + } +} + #[derive(Debug, Error)] #[cfg_attr(test, derive(PartialEq))] pub enum ErrorKind { @@ -14,8 +56,8 @@ pub enum ErrorKind { InvalidProfile(SourceMetadata, String), #[error("Invalid version: {1}")] InvalidVersion(SourceMetadata, u64), - #[error("Unexpected token: {0}")] - InvalidToken(Token), + #[error("Expected {}, found {0}", join_with_comma(.1))] + InvalidToken(Token, Vec), #[error("Not implemented {0}")] NotImplemented(&'static str), #[error("Unknown variable: {1}")] @@ -43,7 +85,7 @@ impl ErrorKind { | ErrorKind::UnknownLayoutQualifier(metadata, _) | ErrorKind::SemanticError(metadata, _) | ErrorKind::UnknownField(metadata, _) => Some(metadata), - ErrorKind::InvalidToken(ref token) => Some(token.meta), + ErrorKind::InvalidToken(ref token, _) => Some(token.meta), _ => None, } } diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index ff9b392792..237a8c792f 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -11,9 +11,9 @@ use super::{ Program, }; use crate::{ - arena::Handle, Arena, ArraySize, BinaryOperator, Block, Constant, ConstantInner, Expression, - Function, FunctionResult, ResourceBinding, ScalarValue, Statement, StorageClass, StructMember, - SwitchCase, Type, TypeInner, UnaryOperator, + arena::Handle, front::glsl::error::ExpectedToken, Arena, ArraySize, BinaryOperator, Block, + Constant, ConstantInner, Expression, Function, FunctionResult, ResourceBinding, ScalarValue, + Statement, StorageClass, StructMember, SwitchCase, Type, TypeInner, UnaryOperator, }; use core::convert::TryFrom; use std::{iter::Peekable, mem}; @@ -38,7 +38,10 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { match token.value { TokenValue::Identifier(name) => Ok((name, token.meta)), - _ => Err(ErrorKind::InvalidToken(token)), + _ => Err(ErrorKind::InvalidToken( + token, + vec![ExpectedToken::Identifier], + )), } } @@ -46,7 +49,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let token = self.bump()?; if token.value != value { - Err(ErrorKind::InvalidToken(token)) + Err(ErrorKind::InvalidToken(token, vec![value.into()])) } else { Ok(token) } @@ -90,7 +93,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { 440 | 450 | 460 => self.program.version = i.value as u16, _ => return Err(ErrorKind::InvalidVersion(version.meta, i.value)), }, - _ => return Err(ErrorKind::InvalidToken(version)), + _ => { + return Err(ErrorKind::InvalidToken( + version, + vec![ExpectedToken::IntLiteral], + )) + } } let profile = self.lexer.peek(); @@ -155,7 +163,16 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { Some(ty) => Some(*ty), None => return Err(ErrorKind::UnknownType(token.meta, ident)), }, - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![ + TokenValue::Void.into(), + TokenValue::Struct.into(), + ExpectedToken::TypeName, + ], + )) + } }; let size = self.parse_array_specifier()?; @@ -363,7 +380,10 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { Ok(()) } // TODO: handle Shared? - _ => Err(ErrorKind::InvalidToken(token)), + _ => Err(ErrorKind::InvalidToken( + token, + vec![ExpectedToken::Identifier], + )), } } @@ -405,7 +425,13 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let token = self.bump()?; match token.value { TokenValue::Semicolon if self.program.version == 460 => Ok(()), - _ => Err(ErrorKind::InvalidToken(token)), + _ => { + let expected = match self.program.version { + 460 => vec![TokenValue::Semicolon.into(), ExpectedToken::EOF], + _ => vec![ExpectedToken::EOF], + }; + Err(ErrorKind::InvalidToken(token, expected)) + } } } else { Ok(()) @@ -479,7 +505,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { meta = meta.union(&token.meta); break; } - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![TokenValue::Comma.into(), TokenValue::RightBrace.into()], + )) + } } } @@ -534,7 +565,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let name = match token.value { TokenValue::Semicolon => break, TokenValue::Identifier(name) => name, - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![ExpectedToken::Identifier, TokenValue::Semicolon.into()], + )) + } }; let mut meta = token.meta; @@ -581,7 +617,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { match token.value { TokenValue::Semicolon => break, TokenValue::Comma => {} - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![TokenValue::Comma.into(), TokenValue::Semicolon.into()], + )) + } } } @@ -697,7 +738,17 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { Ok(true) } - _ => Err(ErrorKind::InvalidToken(token)), + _ if external => Err(ErrorKind::InvalidToken( + token, + vec![ + TokenValue::LeftBrace.into(), + TokenValue::Semicolon.into(), + ], + )), + _ => Err(ErrorKind::InvalidToken( + token, + vec![TokenValue::Semicolon.into()], + )), }; } // Pass the token to the init_declator_list parser @@ -772,7 +823,10 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { Ok(true) } - _ => Err(ErrorKind::InvalidToken(token)), + _ => Err(ErrorKind::InvalidToken( + token, + vec![ExpectedToken::Identifier, TokenValue::Semicolon.into()], + )), } } } else { @@ -820,7 +874,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { Some(name) } - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![ExpectedToken::Identifier, TokenValue::Semicolon.into()], + )) + } }; meta = meta.union(&token.meta); @@ -914,7 +973,17 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { return Ok(expr); } - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![ + TokenValue::LeftParen.into(), + ExpectedToken::IntLiteral, + ExpectedToken::FloatLiteral, + ExpectedToken::BoolLiteral, + ], + )) + } }; let handle = self.program.module.constants.append(Constant { @@ -949,7 +1018,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { *meta = meta.union(&token.meta); break; } - _ => return Err(ErrorKind::InvalidToken(token)), + _ => { + return Err(ErrorKind::InvalidToken( + token, + vec![TokenValue::Comma.into(), TokenValue::RightParen.into()], + )) + } } } } @@ -1420,7 +1494,16 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { self.bump()?; break; } - _ => return Err(ErrorKind::InvalidToken(self.bump()?)), + _ => { + return Err(ErrorKind::InvalidToken( + self.bump()?, + vec![ + TokenValue::Case.into(), + TokenValue::Default.into(), + TokenValue::RightBrace.into(), + ], + )) + } } } diff --git a/src/front/glsl/parser_tests.rs b/src/front/glsl/parser_tests.rs index 9bed09ffa5..81107171e3 100644 --- a/src/front/glsl/parser_tests.rs +++ b/src/front/glsl/parser_tests.rs @@ -4,6 +4,7 @@ use super::lex::Lexer; use super::parser; use super::{ast::Profile, error::ErrorKind}; use super::{ast::Program, SourceMetadata}; +use crate::front::glsl::error::ExpectedToken; use crate::{ front::glsl::{token::TokenValue, Token}, ShaderStage, @@ -50,10 +51,13 @@ fn version() { parse_program("#version 450\nvoid f(){} #version 450", &entry_points) .err() .unwrap(), - ErrorKind::InvalidToken(Token { - value: TokenValue::Unknown(PreprocessorError::UnexpectedHash), - meta: SourceMetadata { start: 24, end: 25 } - }) + ErrorKind::InvalidToken( + Token { + value: TokenValue::Unknown(PreprocessorError::UnexpectedHash), + meta: SourceMetadata { start: 24, end: 25 } + }, + vec![ExpectedToken::EOF] + ) ); // valid versions From 2a1f69286d4d7be8c606f48e139c64396b0282f4 Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Tue, 1 Jun 2021 21:03:40 +0200 Subject: [PATCH 2/3] [glsl-in] add source metadata to VariableAlreadyDeclared error --- src/front/glsl/error.rs | 6 ++++-- src/front/glsl/variables.rs | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/front/glsl/error.rs b/src/front/glsl/error.rs index bb2baa972a..420f6a2e9f 100644 --- a/src/front/glsl/error.rs +++ b/src/front/glsl/error.rs @@ -69,8 +69,8 @@ pub enum ErrorKind { #[error("Unknown layout qualifier: {1}")] UnknownLayoutQualifier(SourceMetadata, String), #[cfg(feature = "glsl-validate")] - #[error("Variable already declared: {0}")] - VariableAlreadyDeclared(String), + #[error("Variable already declared: {1}")] + VariableAlreadyDeclared(SourceMetadata, String), #[error("{1}")] SemanticError(SourceMetadata, Cow<'static, str>), } @@ -85,6 +85,8 @@ impl ErrorKind { | ErrorKind::UnknownLayoutQualifier(metadata, _) | ErrorKind::SemanticError(metadata, _) | ErrorKind::UnknownField(metadata, _) => Some(metadata), + #[cfg(feature = "glsl-validate")] + ErrorKind::VariableAlreadyDeclared(metadata, _) => Some(metadata), ErrorKind::InvalidToken(ref token, _) => Some(token.meta), _ => None, } diff --git a/src/front/glsl/variables.rs b/src/front/glsl/variables.rs index 0108adf8f8..205514f036 100644 --- a/src/front/glsl/variables.rs +++ b/src/front/glsl/variables.rs @@ -510,18 +510,19 @@ impl Program<'_> { &mut self, ctx: &mut Context, body: &mut Block, + #[cfg_attr(not(feature = "glsl-validate"), allow(unused_variables))] VarDeclaration { qualifiers, ty, name, init, - .. + meta, }: VarDeclaration, ) -> Result, ErrorKind> { #[cfg(feature = "glsl-validate")] if let Some(ref name) = name { if ctx.lookup_local_var_current_scope(name).is_some() { - return Err(ErrorKind::VariableAlreadyDeclared(name.clone())); + return Err(ErrorKind::VariableAlreadyDeclared(meta, name.clone())); } } From 6b75cdd4639c0752a2e5f6ec3878da73dc43925f Mon Sep 17 00:00:00 2001 From: Jakob Hellermann Date: Sun, 13 Jun 2021 10:54:03 +0200 Subject: [PATCH 3/3] [cli] add codespan_reporting to glsl error --- cli/src/main.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 14d4c11853..3e04b055b0 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -20,8 +20,15 @@ trait PrettyResult { } fn print_err(error: impl Error) { - eprintln!("{}:", error); + eprint!("{}", error); + let mut e = error.source(); + if e.is_some() { + eprintln!(": "); + } else { + eprintln!(); + } + while let Some(source) = e { eprintln!("\t{}", source); e = source.source(); @@ -92,7 +99,7 @@ fn main() { } let input_path = match input_path { - Some(ref string) => string, + Some(ref string) => Path::new(string), None => { println!("Call with []"); return; @@ -135,7 +142,11 @@ fn main() { defines: Default::default(), }, ) - .unwrap_pretty() + .unwrap_or_else(|err| { + let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str); + emit_glsl_parser_error(err, filename.unwrap_or("glsl"), &input); + std::process::exit(1); + }) } "frag" => { let input = fs::read_to_string(input_path).unwrap(); @@ -148,7 +159,11 @@ fn main() { defines: Default::default(), }, ) - .unwrap_pretty() + .unwrap_or_else(|err| { + let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str); + emit_glsl_parser_error(err, filename.unwrap_or("glsl"), &input); + std::process::exit(1); + }) } "comp" => { let input = fs::read_to_string(input_path).unwrap(); @@ -161,7 +176,11 @@ fn main() { defines: Default::default(), }, ) - .unwrap_pretty() + .unwrap_or_else(|err| { + let filename = input_path.file_name().and_then(std::ffi::OsStr::to_str); + emit_glsl_parser_error(err, filename.unwrap_or("glsl"), &input); + std::process::exit(1); + }) } other => panic!("Unknown input extension: {}", other), }; @@ -274,3 +293,26 @@ fn main() { } } } + +use codespan_reporting::{ + diagnostic::{Diagnostic, Label}, + files::SimpleFile, + term::{ + self, + termcolor::{ColorChoice, StandardStream}, + }, +}; + +pub fn emit_glsl_parser_error(err: naga::front::glsl::ParseError, filename: &str, source: &str) { + let diagnostic = match err.kind.metadata() { + Some(metadata) => Diagnostic::error() + .with_message(err.kind.to_string()) + .with_labels(vec![Label::primary((), metadata.start..metadata.end)]), + None => Diagnostic::error().with_message(err.kind.to_string()), + }; + + let files = SimpleFile::new(filename, source); + let config = codespan_reporting::term::Config::default(); + let writer = StandardStream::stderr(ColorChoice::Auto); + term::emit(&mut writer.lock(), &config, &files, &diagnostic).expect("cannot write error"); +}