From 2b59afd7589e3b686d4b3e810530371a099d5746 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 16:23:57 +0100 Subject: [PATCH 01/30] well it compiles --- src/compose/error.rs | 14 +- src/compose/ident_identifier.rs | 356 +++++++++++++++++++++++++ src/compose/mod.rs | 172 ++----------- src/compose/preprocess.rs | 443 ++++++++++++++------------------ 4 files changed, 575 insertions(+), 410 deletions(-) create mode 100644 src/compose/ident_identifier.rs diff --git a/src/compose/error.rs b/src/compose/error.rs index 112d080..48d446d 100644 --- a/src/compose/error.rs +++ b/src/compose/error.rs @@ -9,7 +9,7 @@ use thiserror::Error; use tracing::trace; use super::{ - preprocess::{PreprocessOutput, PreprocessorMetaData}, + preprocess::PreprocessOutput, Composer, ShaderDefValue, }; use crate::{compose::SPAN_SHIFT, redirect::RedirectError}; @@ -42,7 +42,7 @@ impl ErrSource { let raw_source = &composer.module_sets.get(name).unwrap().sanitized_source; let Ok(PreprocessOutput { preprocessed_source: source, - meta: PreprocessorMetaData { imports, .. }, + .. }) = composer .preprocessor .preprocess(raw_source, defs, composer.validate) @@ -50,10 +50,6 @@ impl ErrSource { return Default::default() }; - let Ok(source) = composer - .substitute_shader_string(&source, &imports) - else { return Default::default() }; - Cow::Owned(source) } ErrSource::Constructing { source, .. } => Cow::Borrowed(source), @@ -77,6 +73,8 @@ pub struct ComposerError { #[derive(Debug, Error)] pub enum ComposerErrorInner { + #[error("{0}")] + ImportParseError(String, usize), #[error("required import '{0}' not found")] ImportNotFound(String, usize), #[error("{0}")] @@ -215,6 +213,10 @@ impl ComposerError { vec![Label::primary((), *pos..*pos)], vec![format!("missing import '{msg}'")], ), + ComposerErrorInner::ImportParseError(msg, pos) => ( + vec![Label::primary((), *pos..*pos)], + vec![format!("invalid import spec: '{msg}'")], + ), ComposerErrorInner::WgslParseError(e) => ( e.labels() .map(|(range, msg)| { diff --git a/src/compose/ident_identifier.rs b/src/compose/ident_identifier.rs new file mode 100644 index 0000000..1e0308c --- /dev/null +++ b/src/compose/ident_identifier.rs @@ -0,0 +1,356 @@ +use std::collections::{VecDeque, HashMap}; + +use super::{ImportDefWithOffset, ImportDefinition}; + +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub enum Token<'a> { + Identifier(&'a str, usize), + Other(char, usize), + Whitespace(&'a str, usize), +} + +impl<'a> Token<'a> { + fn pos(&self) -> usize { + match self { + Token::Identifier(_, pos) | + Token::Other(_, pos) | + Token::Whitespace(_, pos) => *pos + } + } + + fn identifier(&self) -> Option<&str> { + match self { + Token::Identifier(ident, _) => Some(ident), + _ => None, + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum TokenKind { + Identifier, + Whitespace, +} + +// a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens +// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters +pub struct Tokenizer<'a> { + tokens: VecDeque>, +} + +impl<'a> Tokenizer<'a> { + pub fn new(src: &'a str, emit_whitespace: bool) -> Self{ + let mut tokens = VecDeque::default(); + let mut current_token_start = 0; + let mut current_token = None; + + for (ix, char) in src.chars().enumerate() { + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { + continue; + } + tokens.push_back(Token::Identifier(&src[current_token_start..ix], current_token_start)); + } + TokenKind::Whitespace => { + if char.is_whitespace() { + continue; + } + tokens.push_back(Token::Whitespace(&src[current_token_start..ix], current_token_start)); + } + }; + + current_token_start = ix; + current_token = None; + } + + if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { + current_token = Some(TokenKind::Identifier); + current_token_start = ix; + } else if !char.is_whitespace() { + tokens.push_back(Token::Other(char, current_token_start)); + } else if char.is_whitespace() && emit_whitespace { + current_token = Some(TokenKind::Whitespace); + current_token_start = ix; + } + } + + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + tokens.push_back(Token::Identifier(&src[current_token_start..src.len()], current_token_start)); + } + TokenKind::Whitespace => { + tokens.push_back(Token::Whitespace(&src[current_token_start..src.len()], current_token_start)); + } + }; + } + + Self { + tokens, + } + } +} + +impl<'a> Iterator for Tokenizer<'a> { + type Item = Token<'a>; + + fn next(&mut self) -> Option { + let tok = self.tokens.pop_front(); + tok + } +} + +pub fn parse_imports(input: &str) -> Result, (&str, usize)> { + let mut tokens = Tokenizer::new(input, false).peekable(); + + match tokens.next() { + Some(Token::Other('#', _)) => (), + Some(other) => return Err(("expected `#import`", other.pos())), + None => return Err(("expected #import", input.len())), + }; + match tokens.next() { + Some(Token::Identifier("import", _)) => (), + Some(other) => return Err(("expected `#import`", other.pos())), + None => return Err(("expected `#import`", input.len())), + }; + + let mut output = HashMap::default(); + let mut stack = Vec::default(); + let mut current = String::default(); + let mut as_name = None; + let mut is_deprecated_itemlist = false; + + loop { + match tokens.peek() { + Some(Token::Identifier(ident, _)) => { + current.push_str(ident); + tokens.next(); + + if tokens.peek().and_then(Token::identifier) == Some("as") { + let pos = tokens.next().unwrap().pos(); + let Some(Token::Identifier(name, _)) = tokens.next() else { + return Err(("expected identifier after `as`", pos)); + }; + + as_name = Some(name); + } + + // support deprecated #import mod item + if let Some(Token::Identifier(..)) = tokens.peek() { + is_deprecated_itemlist = true; + stack.push(format!("{}::", current)); + current = String::default(); + as_name = None; + } + + continue; + } + Some(Token::Other('{', pos)) => { + if !current.ends_with("::") { + return Err(("open brace must follow `::`", *pos)); + } + stack.push(current); + current = String::default(); + as_name = None; + } + Some(Token::Other(',', _)) | + Some(Token::Other('}', _)) | + None => { + if !current.is_empty() { + let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| current.rsplit_once("::").map(|(_, name)| name.to_owned()).unwrap_or(current.clone())); + output.insert(used_name, format!("{}{}", stack.join(""), current)); + current = String::default(); + as_name = None; + } + + if let Some(Token::Other('}', pos)) = tokens.peek() { + if stack.pop().is_none() { + return Err(("close brace without open", *pos)); + } + } + + if tokens.peek().is_none() { + break; + } + } + Some(Token::Other(_, pos)) => return Err(("unexpected token", *pos)), + Some(Token::Whitespace(..)) => unreachable!(), + } + + tokens.next(); + } + + if !stack.is_empty() && !(is_deprecated_itemlist && stack.len() == 1) { + return Err(("missing close brace", input.len())); + } + + Ok(output) +} + +const DECORATION_PRE: &str = "_naga_oil_mod_"; +const DECORATION_POST: &str = "X"; // just to ensure we don't end with a number + +fn decorate(module: &str, item: &str) -> String { + let encoded = data_encoding::BASE32_NOPAD.encode(module.as_bytes()); + format!("{item}{DECORATION_PRE}{encoded}{DECORATION_POST}") +} + +pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &HashMap, used_imports: &mut HashMap) -> String { + let tokens = Tokenizer::new(input, true); + let mut output = String::with_capacity(input.len()); + let mut in_substitution_position = true; + + for token in tokens { + match token { + Token::Identifier(ident, token_pos) => { + if in_substitution_position { + let (first, residual) = ident.split_once("::").unwrap_or((ident, "")); + let mut full_path = declared_imports.get(first).cloned().unwrap_or(first.to_owned()); + if !residual.is_empty() { + full_path.push_str("::"); + full_path.push_str(residual); + } + + if let Some((module, item)) = full_path.rsplit_once("::") { + used_imports.entry(module.to_owned()).or_insert_with(|| { + ImportDefWithOffset { definition: ImportDefinition { import: module.to_owned(), ..Default::default() }, offset: offset + token_pos } + }).definition.items.push(item.to_owned()); + output.push_str(&decorate(module, item)) + } else { + output.push_str(&full_path); + } + } else { + output.push_str(ident); + } + }, + Token::Other(other, _) => { + output.push(other); + if other == '.' || other == '@' { + in_substitution_position = false; + continue; + } + } + Token::Whitespace(ws, _) => output.push_str(ws), + } + + in_substitution_position = true; + } + + output +} + +#[test] +fn import_tokens() { + let input = r" + #import a::b + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([("b".to_owned(), "a::b".to_owned())]))); + + let input = r" + #import a::{b, c} + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("b".to_owned(), "a::b".to_owned()), + ("c".to_owned(), "a::c".to_owned()), + ]))); + + let input = r" + #import a::{b as d, c} + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("d".to_owned(), "a::b".to_owned()), + ("c".to_owned(), "a::c".to_owned()), + ]))); + + let input = r" + #import a::{b::{c, d}, e} + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("c".to_owned(), "a::b::c".to_owned()), + ("d".to_owned(), "a::b::d".to_owned()), + ("e".to_owned(), "a::e".to_owned()), + ]))); + + let input = r" + #import a::b::{c, d}, e + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("c".to_owned(), "a::b::c".to_owned()), + ("d".to_owned(), "a::b::d".to_owned()), + ("e".to_owned(), "e".to_owned()), + ]))); + + let input = r" + #import a, b + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("a".to_owned(), "a".to_owned()), + ("b".to_owned(), "b".to_owned()), + ]))); + + let input = r" + #import a::b c, d + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("c".to_owned(), "a::b::c".to_owned()), + ("d".to_owned(), "a::b::d".to_owned()), + ]))); + + let input = r" + #import a::b c + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("c".to_owned(), "a::b::c".to_owned()), + ]))); + + let input = r" + #import a::b::{c::{d, e}, f, g::{h as i, j}} + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("d".to_owned(), "a::b::c::d".to_owned()), + ("e".to_owned(), "a::b::c::e".to_owned()), + ("f".to_owned(), "a::b::f".to_owned()), + ("i".to_owned(), "a::b::g::h".to_owned()), + ("j".to_owned(), "a::b::g::j".to_owned()), + ]))); + + let input = r" + #import a::b::{ + c::{d, e}, + f, + g::{ + h as i, + j::k::l as m, + } + } + "; + assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ + ("d".to_owned(), "a::b::c::d".to_owned()), + ("e".to_owned(), "a::b::c::e".to_owned()), + ("f".to_owned(), "a::b::f".to_owned()), + ("i".to_owned(), "a::b::g::h".to_owned()), + ("m".to_owned(), "a::b::g::j::k::l".to_owned()), + ]))); + + let input = r" + #import a::b::{ + "; + assert!(parse_imports(input).is_err()); + + let input = r" + #import a::b::{c}} + "; + assert!(parse_imports(input).is_err()); + + let input = r" + #import a::b::{c}} + "; + assert!(parse_imports(input).is_err()); + + let input = r" + #import a::b{{c,d}} + "; + assert!(parse_imports(input).is_err()); +} diff --git a/src/compose/mod.rs b/src/compose/mod.rs index f056079..d62bedc 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -143,6 +143,7 @@ pub mod comment_strip_iter; pub mod error; pub mod preprocess; mod test; +pub mod ident_identifier; #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug, Default)] pub enum ShaderLanguage { @@ -262,6 +263,8 @@ pub struct ComposableModuleDefinition { modules: HashMap, // used in spans when this module is included module_index: usize, + // preprocessor meta data + // metadata: PreprocessorMetaData, } impl ComposableModuleDefinition { @@ -291,14 +294,7 @@ impl ComposableModuleDefinition { #[derive(Debug, Clone, Default)] pub struct ImportDefinition { pub import: String, - pub as_name: Option, - pub items: Option>, -} - -impl ImportDefinition { - fn as_name(&self) -> &str { - self.as_name.as_deref().unwrap_or(self.import.as_str()) - } + pub items: Vec, } #[derive(Debug, Clone)] @@ -450,99 +446,6 @@ impl Composer { substituted_source.into_owned() } - fn substitute_shader_string( - &self, - source: &str, - imports: &[ImportDefWithOffset], - ) -> Result { - // sort imports by decreasing length so we don't accidentally replace substrings of a longer import - let mut imports = imports.to_vec(); - imports.sort_by_key(|import| usize::MAX - import.definition.as_name().len()); - - let mut imported_items = HashMap::new(); - let mut substituted_source = source.to_owned(); - - for import in imports { - match import.definition.items { - Some(items) => { - // gather individual imported items - for item in &items { - imported_items.insert( - item.clone(), - format!("{}{}", Self::decorate(&import.definition.import), item), - ); - } - } - None => { - // replace the module name directly - substituted_source = substituted_source.replace( - format!("{}::", import.definition.as_name()).as_str(), - &Self::decorate(&import.definition.import), - ); - } - } - } - - // map individually imported items - let mut item_substituted_source = String::default(); - let mut current_word = String::default(); - let mut line_is_directive = None; - let mut is_valid_import_substitution_point = true; - - for char in substituted_source.chars() { - if !current_word.is_empty() { - if unicode_ident::is_xid_continue(char) { - current_word.push(char); - continue; - } - - let mut output = ¤t_word; - - // substitute current word if we are not writing a directive (e.g. `#import xyz`) - if is_valid_import_substitution_point { - if let Some(replacement) = imported_items.get(¤t_word) { - output = replacement; - } - } - - item_substituted_source += output; - current_word.clear(); - } - - // set current directive state - if char == '\r' || char == '\n' { - // new line -> could be - line_is_directive = None; - } else if line_is_directive.is_none() { - line_is_directive = match char { - // first non-ws char is a #, this is a directive - '#' => Some(true), - // whitespace, still unknown - ' ' | '\t' => None, - // non-whitespace and not a '#', not a directive - _ => Some(false), - } - } - - if unicode_ident::is_xid_start(char) { - current_word.push(char); - } else { - item_substituted_source.push(char); - // we should only substitute global names - // '.' -> avoid substituting members with name == import item - // '@' -> avoid substituting annotations - if char == '.' || char == '@' { - is_valid_import_substitution_point = false; - } else { - is_valid_import_substitution_point = !line_is_directive.unwrap_or(false); - } - } - } - substituted_source = item_substituted_source; - - Ok(substituted_source) - } - fn naga_to_string( &self, naga_module: &mut naga::Module, @@ -915,7 +818,7 @@ impl Composer { let PreprocessOutput { preprocessed_source: source, - meta: PreprocessorMetaData { imports, .. }, + imports } = self .preprocessor .preprocess( @@ -925,10 +828,6 @@ impl Composer { ) .map_err(wrap_err)?; - let source = self - .substitute_shader_string(&source, &imports) - .map_err(wrap_err)?; - let mut imports: Vec<_> = imports .into_iter() .map(|import_with_offset| import_with_offset.definition) @@ -1337,10 +1236,6 @@ impl Composer { return; } - if import.items.is_none() { - already_added.insert(import.import.clone()); - } - let import_module_set = self.module_sets.get(&import.import).unwrap(); let module = import_module_set.get_module(shader_defs).unwrap(); @@ -1351,7 +1246,7 @@ impl Composer { Self::add_composable_data( derived, module, - import.items.as_ref(), + Some(&import.items), import_module_set.module_index << SPAN_SHIFT, header, ); @@ -1373,7 +1268,6 @@ impl Composer { defs: shader_defs.clone(), }, })? - .meta .imports; self.ensure_imports(imports.iter().map(|import| &import.definition), shader_defs)?; @@ -1497,13 +1391,12 @@ impl Composer { let substituted_source = self.sanitize_and_set_auto_bindings(source); - let ( - PreprocessorMetaData { - name: module_name, - mut imports, - }, - _, - ) = self + let PreprocessorMetaData { + name: module_name, + mut imports, + mut effective_defs, + .. + } = self .preprocessor .get_preprocessor_metadata(&substituted_source, false) .map_err(|inner| ComposerError { @@ -1544,7 +1437,6 @@ impl Composer { }), ); - let mut effective_defs = HashSet::new(); for import in &imports { // we require modules already added so that we can capture the shader_defs that may impact us by impacting our dependencies let module_set = self @@ -1570,9 +1462,6 @@ impl Composer { ); } - // record our explicit effective shader_defs - effective_defs.extend(self.preprocessor.effective_defs(source)); - // remove defs that are already specified through our imports effective_defs.retain(|name| !shader_defs.contains_key(name)); @@ -1637,7 +1526,7 @@ impl Composer { let sanitized_source = self.sanitize_and_set_auto_bindings(source); - let (_, defines) = self + let PreprocessorMetaData { name, defines, imports, .. } = self .preprocessor .get_preprocessor_metadata(&sanitized_source, true) .map_err(|inner| ComposerError { @@ -1650,32 +1539,7 @@ impl Composer { })?; shader_defs.extend(defines.into_iter()); - let PreprocessOutput { - preprocessed_source, - meta: PreprocessorMetaData { name, imports }, - } = self - .preprocessor - .preprocess(&sanitized_source, &shader_defs, false) - .map_err(|inner| ComposerError { - inner, - source: ErrSource::Constructing { - path: file_path.to_owned(), - source: sanitized_source.to_owned(), - offset: 0, - }, - })?; - let name = name.unwrap_or_default(); - let substituted_source = self - .substitute_shader_string(&sanitized_source, &imports) - .map_err(|inner| ComposerError { - inner, - source: ErrSource::Constructing { - path: file_path.to_owned(), - source: sanitized_source.to_owned(), - offset: 0, - }, - })?; // make sure imports have been added // and gather additional defs specified at module level @@ -1720,7 +1584,7 @@ impl Composer { let definition = ComposableModuleDefinition { name, - sanitized_source: substituted_source, + sanitized_source: sanitized_source.clone(), language: shader_type.into(), file_path: file_path.to_owned(), module_index: 0, @@ -1797,7 +1661,7 @@ impl Composer { inner: e.into(), source: ErrSource::Constructing { path: file_path.to_owned(), - source: preprocessed_source.clone(), + source: sanitized_source.clone(), offset: composable.start_offset, }, })?; @@ -1810,7 +1674,7 @@ impl Composer { inner: e.into(), source: ErrSource::Constructing { path: file_path.to_owned(), - source: preprocessed_source.clone(), + source: sanitized_source.clone(), offset: composable.start_offset, }, })?; @@ -1854,7 +1718,7 @@ impl Composer { } None => ErrSource::Constructing { path: file_path.to_owned(), - source: preprocessed_source, + source: sanitized_source, offset: composable.start_offset, }, }; @@ -1882,7 +1746,7 @@ pub fn get_preprocessor_data( Vec, HashMap, ) { - let (PreprocessorMetaData { name, imports }, defines) = PREPROCESSOR + let PreprocessorMetaData { name, imports, defines, .. } = PREPROCESSOR .get_preprocessor_metadata(source, true) .unwrap(); ( diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 5911b79..cd88936 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -1,11 +1,10 @@ use std::collections::{HashMap, HashSet}; use regex::Regex; -use tracing::warn; use super::{ comment_strip_iter::CommentReplaceExt, ComposerErrorInner, ImportDefWithOffset, - ImportDefinition, ShaderDefValue, + ShaderDefValue, ident_identifier::{parse_imports, substitute_identifiers}, }; #[derive(Debug)] @@ -18,10 +17,7 @@ pub struct Preprocessor { endif_regex: Regex, def_regex: Regex, def_regex_delimited: Regex, - import_custom_path_as_regex: Regex, - import_custom_path_regex: Regex, - import_items_regex: Regex, - identifier_regex: Regex, + import_regex: Regex, define_import_path_regex: Regex, define_shader_def_regex: Regex, } @@ -40,14 +36,7 @@ impl Default for Preprocessor { endif_regex: Regex::new(r"^\s*#\s*endif").unwrap(), def_regex: Regex::new(r"#\s*([\w|\d|_]+)").unwrap(), def_regex_delimited: Regex::new(r"#\s*\{([\w|\d|_]+)\}").unwrap(), - import_custom_path_as_regex: Regex::new(r"^\s*#\s*import\s+([^\s]+)\s+as\s+([^\s]+)") - .unwrap(), - import_custom_path_regex: Regex::new(r"^\s*#\s*import\s+([^\s]+)").unwrap(), - import_items_regex: Regex::new( - r"^\s*#\s*import\s+([^\s]+)\s+((?:[\w|\d|_]+)(?:\s*,\s*[\w|\d|_]+)*)", - ) - .unwrap(), - identifier_regex: Regex::new(r"([\w|\d|_]+)").unwrap(), + import_regex: Regex::new(r"^\s*#\s*import\s").unwrap(), define_import_path_regex: Regex::new(r"^\s*#\s*define_import_path\s+([^\s]+)").unwrap(), define_shader_def_regex: Regex::new(r"^\s*#\s*define\s+([\w|\d|_]+)\s*([-\w|\d]+)?") .unwrap(), @@ -59,6 +48,8 @@ impl Default for Preprocessor { pub struct PreprocessorMetaData { pub name: Option, pub imports: Vec, + pub defines: HashMap, + pub effective_defs: HashSet, } enum ScopeLevel { @@ -136,202 +127,174 @@ impl Scope { #[derive(Debug)] pub struct PreprocessOutput { pub preprocessed_source: String, - pub meta: PreprocessorMetaData, + pub imports: Vec, } impl Preprocessor { + fn check_scope<'a>( + &self, + shader_defs: &HashMap, + line: &'a str, + scope: Option<&mut Scope>, + offset: usize, + ) -> Result<(bool, Option<&'a str>), ComposerErrorInner> { + if let Some(cap) = self.ifdef_regex.captures(line) { + let is_else = cap.get(1).is_some(); + let def = cap.get(2).unwrap().as_str(); + let cond = shader_defs.contains_key(def); + scope.map_or(Ok(()), |scope| scope.branch(is_else, cond, offset))?; + return Ok((true, Some(def))); + } else if let Some(cap) = self.ifndef_regex.captures(line) { + let is_else = cap.get(1).is_some(); + let def = cap.get(2).unwrap().as_str(); + let cond = !shader_defs.contains_key(def); + scope.map_or(Ok(()), |scope| scope.branch(is_else, cond, offset))?; + return Ok((true, Some(def))); + } else if let Some(cap) = self.ifop_regex.captures(line) { + let is_else = cap.get(1).is_some(); + let def = cap.get(2).unwrap().as_str(); + let op = cap.get(3).unwrap(); + let val = cap.get(4).unwrap(); + + if scope.is_none() { + // don't try to evaluate if we don't have a scope + return Ok((true, Some(def))); + } + + fn act_on( + a: T, + b: T, + op: &str, + pos: usize, + ) -> Result { + match op { + "==" => Ok(a == b), + "!=" => Ok(a != b), + ">" => Ok(a > b), + ">=" => Ok(a >= b), + "<" => Ok(a < b), + "<=" => Ok(a <= b), + _ => Err(ComposerErrorInner::UnknownShaderDefOperator { + pos, + operator: op.to_string(), + }), + } + } + + let def_value = + shader_defs + .get(def) + .ok_or(ComposerErrorInner::UnknownShaderDef { + pos: offset, + shader_def_name: def.to_string(), + })?; + + let invalid_def = |ty: &str| { + ComposerErrorInner::InvalidShaderDefComparisonValue { + pos: offset, + shader_def_name: def.to_string(), + value: val.as_str().to_string(), + expected: ty.to_string(), + } + }; + + let new_scope = match def_value { + ShaderDefValue::Bool(def_value) => { + let val = val.as_str().parse().map_err(|_| invalid_def("bool"))?; + act_on(*def_value, val, op.as_str(), offset)? + } + ShaderDefValue::Int(def_value) => { + let val = val.as_str().parse().map_err(|_| invalid_def("int"))?; + act_on(*def_value, val, op.as_str(), offset)? + } + ShaderDefValue::UInt(def_value) => { + let val = val.as_str().parse().map_err(|_| invalid_def("uint"))?; + act_on(*def_value, val, op.as_str(), offset)? + } + }; + + scope.map_or(Ok(()), |scope| scope.branch(is_else, new_scope, offset))?; + return Ok((true, Some(def))); + } else if self.else_regex.is_match(line) { + scope.map_or(Ok(()), |scope| scope.branch(true, true, offset))?; + return Ok((true, None)); + } else if self.endif_regex.is_match(line) { + scope.map_or(Ok(()), |scope| scope.pop(offset))?; + return Ok((true, None)); + } + + Ok((false, None)) + } + // process #if[(n)?def]? / #else / #endif preprocessor directives, // strip module name and imports // also strip "#version xxx" + // replace items with resolved decorated names pub fn preprocess( &self, shader_str: &str, shader_defs: &HashMap, - mut validate_len: bool, + validate_len: bool, ) -> Result { - let mut imports = Vec::new(); - + let mut declared_imports = HashMap::new(); + let mut used_imports = HashMap::new(); let mut scope = Scope::new(); let mut final_string = String::new(); - let mut name = None; let mut offset = 0; - let mut at_start = true; - #[cfg(debug)] let len = shader_str.len(); // this code broadly stolen from bevy_render::ShaderProcessor - for (line, original_line) in shader_str - .lines() + let mut lines = shader_str.lines(); + let mut lines = lines .replace_comments() .zip(shader_str.lines()) - { - let line = &line; + .peekable(); + while let Some((mut line, original_line)) = lines.next() { let mut output = false; - let mut still_at_start = false; - if line.is_empty() || line.chars().all(|c| c.is_ascii_whitespace()) { - still_at_start = true; - } - if let Some(cap) = self.version_regex.captures(line) { + if let Some(cap) = self.version_regex.captures(&line) { let v = cap.get(1).unwrap().as_str(); if v != "440" && v != "450" { return Err(ComposerErrorInner::GlslInvalidVersion(offset)); } - } else if let Some(cap) = self.ifdef_regex.captures(line) { - let is_else = cap.get(1).is_some(); - let cond = shader_defs.contains_key(cap.get(2).unwrap().as_str()); - scope.branch(is_else, cond, offset)?; - } else if let Some(cap) = self.ifndef_regex.captures(line) { - let is_else = cap.get(1).is_some(); - let cond = !shader_defs.contains_key(cap.get(2).unwrap().as_str()); - scope.branch(is_else, cond, offset)?; - } else if let Some(cap) = self.ifop_regex.captures(line) { - let is_else = cap.get(1).is_some(); - let def = cap.get(2).unwrap(); - let op = cap.get(3).unwrap(); - let val = cap.get(4).unwrap(); - - fn act_on( - a: T, - b: T, - op: &str, - pos: usize, - ) -> Result { - match op { - "==" => Ok(a == b), - "!=" => Ok(a != b), - ">" => Ok(a > b), - ">=" => Ok(a >= b), - "<" => Ok(a < b), - "<=" => Ok(a <= b), - _ => Err(ComposerErrorInner::UnknownShaderDefOperator { - pos, - operator: op.to_string(), - }), - } - } + } else if self.check_scope(shader_defs, &line, Some(&mut scope), offset)?.0 { + // ignore + } else if self.define_import_path_regex.captures(&line).is_some() { + // ignore + } else if self.define_shader_def_regex.captures(&line).is_some() { + // ignore + } else if scope.active() { + if self.import_regex.is_match(&line) { + let mut import_lines = String::default(); + let mut open_count = 0; - let def_value = - shader_defs - .get(def.as_str()) - .ok_or(ComposerErrorInner::UnknownShaderDef { - pos: offset, - shader_def_name: def.as_str().to_string(), - })?; - let new_scope = match def_value { - ShaderDefValue::Bool(def_value) => { - let val = val.as_str().parse().map_err(|_| { - ComposerErrorInner::InvalidShaderDefComparisonValue { - pos: offset, - shader_def_name: def.as_str().to_string(), - value: val.as_str().to_string(), - expected: "bool".to_string(), - } - })?; - act_on(*def_value, val, op.as_str(), offset)? - } - ShaderDefValue::Int(def_value) => { - let val = val.as_str().parse().map_err(|_| { - ComposerErrorInner::InvalidShaderDefComparisonValue { - pos: offset, - shader_def_name: def.as_str().to_string(), - value: val.as_str().to_string(), - expected: "int".to_string(), - } - })?; - act_on(*def_value, val, op.as_str(), offset)? - } - ShaderDefValue::UInt(def_value) => { - let val = val.as_str().parse().map_err(|_| { - ComposerErrorInner::InvalidShaderDefComparisonValue { - pos: offset, - shader_def_name: def.as_str().to_string(), - value: val.as_str().to_string(), - expected: "int".to_string(), - } - })?; - act_on(*def_value, val, op.as_str(), offset)? - } - }; - - scope.branch(is_else, new_scope, offset)?; - } else if self.else_regex.is_match(line) { - scope.branch(true, true, offset)?; - } else if self.endif_regex.is_match(line) { - scope.pop(offset)?; - } else if let Some(cap) = self.define_import_path_regex.captures(line) { - name = Some(cap.get(1).unwrap().as_str().to_string()); - } else if let Some(cap) = self.define_shader_def_regex.captures(line) { - if at_start { - still_at_start = true; + loop { + // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) + final_string.extend(std::iter::repeat(" ").take(line.len())); + offset += line.len() + 1; - let def = cap.get(1).unwrap(); - let name = def.as_str().to_string(); + open_count += line.match_indices('{').count(); + open_count = open_count.saturating_sub(line.match_indices('}').count()); - let value = if let Some(val) = cap.get(2) { - if let Ok(val) = val.as_str().parse::() { - ShaderDefValue::UInt(val) - } else if let Ok(val) = val.as_str().parse::() { - ShaderDefValue::Int(val) - } else if let Ok(val) = val.as_str().parse::() { - ShaderDefValue::Bool(val) - } else { - return Err(ComposerErrorInner::InvalidShaderDefDefinitionValue { - name, - value: val.as_str().to_string(), - pos: offset, - }); + import_lines.push_str(&line); + + if open_count == 0 || lines.peek().is_none() { + break; } - } else { - ShaderDefValue::Bool(true) - }; - match shader_defs.get(name.as_str()) { - Some(current_value) if *current_value == value => (), - _ => return Err(ComposerErrorInner::DefineInModule(offset)), + line = lines.next().unwrap().0; } - } else { - return Err(ComposerErrorInner::DefineInModule(offset)); - } - } else if scope.active() { - if let Some(cap) = self.import_custom_path_as_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: Some(cap.get(2).unwrap().as_str().to_string()), - items: Default::default(), - }, - offset, - }); - } else if let Some(cap) = self.import_items_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: None, - items: Some( - self.identifier_regex - .captures_iter(cap.get(2).unwrap().as_str()) - .map(|ident_cap| ident_cap.get(1).unwrap().as_str().to_owned()) - .collect(), - ), - }, - offset, - }); - } else if let Some(cap) = self.import_custom_path_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: None, - items: Default::default(), - }, - offset, - }); + + declared_imports.extend( + parse_imports(import_lines.as_str()).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))? + ); + output = true; } else { let mut line_with_defs = original_line.to_string(); - for capture in self.def_regex.captures_iter(line) { + for capture in self.def_regex.captures_iter(&line) { let def = capture.get(1).unwrap(); if let Some(def) = shader_defs.get(def.as_str()) { line_with_defs = self @@ -340,7 +303,7 @@ impl Preprocessor { .to_string(); } } - for capture in self.def_regex_delimited.captures_iter(line) { + for capture in self.def_regex_delimited.captures_iter(&line) { let def = capture.get(1).unwrap(); if let Some(def) = shader_defs.get(def.as_str()) { line_with_defs = self @@ -349,15 +312,13 @@ impl Preprocessor { .to_string(); } } - final_string.push_str(&line_with_defs); - let diff = line.len() as i32 - line_with_defs.len() as i32; - if diff > 0 { - final_string.extend(std::iter::repeat(" ").take(diff as usize)); - } else if diff < 0 && validate_len { - // this sucks - warn!("source code map requires shader_def values to be no longer than the corresponding shader_def name, error reporting may not be correct:\noriginal: {}\nreplaced: {}", line, line_with_defs); - validate_len = false; - } + + let item_replaced_line = substitute_identifiers(line_with_defs.as_str(), offset, &declared_imports, &mut used_imports); + + final_string.push_str(&item_replaced_line); + let diff = line.len().saturating_sub(item_replaced_line.len()); + final_string.extend(std::iter::repeat(" ").take(diff as usize)); + offset += original_line.len() + 1; output = true; } } @@ -365,11 +326,9 @@ impl Preprocessor { if !output { // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) final_string.extend(std::iter::repeat(" ").take(line.len())); + offset += line.len() + 1; } final_string.push('\n'); - offset += line.len() + 1; - - at_start &= still_at_start; } scope.finish(offset)?; @@ -379,10 +338,12 @@ impl Preprocessor { let revised_len = final_string.len(); assert_eq!(len, revised_len); } + #[cfg(not(debug))] + let _ = validate_len; Ok(PreprocessOutput { preprocessed_source: final_string, - meta: PreprocessorMetaData { name, imports }, + imports: used_imports.into_values().collect() }) } @@ -391,49 +352,46 @@ impl Preprocessor { &self, shader_str: &str, allow_defines: bool, - ) -> Result<(PreprocessorMetaData, HashMap), ComposerErrorInner> { - let mut imports = Vec::new(); + ) -> Result { + let mut declared_imports = HashMap::default(); + let mut used_imports = HashMap::default(); let mut name = None; let mut offset = 0; let mut defines = HashMap::default(); + let mut effective_defs = HashSet::default(); + + let mut lines = shader_str.lines(); + let mut lines = lines.replace_comments().peekable(); + + while let Some(mut line) = lines.next() { + if let Some(def) = self.check_scope(&HashMap::default(), &line, None, offset)?.1 { + effective_defs.insert(def.to_owned()); + } else if self.import_regex.is_match(&line) { + let mut import_lines = String::default(); + let mut open_count = 0; + + loop { + // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) + offset += line.len() + 1; + + open_count += line.match_indices('{').count(); + open_count = open_count.saturating_sub(line.match_indices('}').count()); + + import_lines.push_str(&line); + + if open_count == 0 || lines.peek().is_none() { + break; + } + + line = lines.next().unwrap(); + } - for line in shader_str.lines().replace_comments() { - let line = &line; - if let Some(cap) = self.import_custom_path_as_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: Some(cap.get(2).unwrap().as_str().to_string()), - items: Default::default(), - }, - offset, - }); - } else if let Some(cap) = self.import_items_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: None, - items: Some( - self.identifier_regex - .captures_iter(cap.get(2).unwrap().as_str()) - .map(|ident_cap| ident_cap.get(1).unwrap().as_str().to_owned()) - .collect(), - ), - }, - offset, - }); - } else if let Some(cap) = self.import_custom_path_regex.captures(line) { - imports.push(ImportDefWithOffset { - definition: ImportDefinition { - import: cap.get(1).unwrap().as_str().to_string(), - as_name: None, - items: Default::default(), - }, - offset, - }); - } else if let Some(cap) = self.define_import_path_regex.captures(line) { + declared_imports.extend( + parse_imports(import_lines.as_str()).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))? + ); + } else if let Some(cap) = self.define_import_path_regex.captures(&line) { name = Some(cap.get(1).unwrap().as_str().to_string()); - } else if let Some(cap) = self.define_shader_def_regex.captures(line) { + } else if let Some(cap) = self.define_shader_def_regex.captures(&line) { if allow_defines { let def = cap.get(1).unwrap(); let name = def.as_str().to_string(); @@ -456,29 +414,14 @@ impl Preprocessor { } else { return Err(ComposerErrorInner::DefineInModule(offset)); } + } else { + substitute_identifiers(&line, offset, &declared_imports, &mut used_imports); } offset += line.len() + 1; } - Ok((PreprocessorMetaData { name, imports }, defines)) - } - - pub fn effective_defs(&self, source: &str) -> HashSet { - let mut effective_defs = HashSet::default(); - - for line in source.lines().replace_comments() { - if let Some(cap) = self.ifdef_regex.captures(&line) { - let def = cap.get(2).unwrap(); - effective_defs.insert(def.as_str().to_owned()); - } - if let Some(cap) = self.ifndef_regex.captures(&line) { - let def = cap.get(2).unwrap(); - effective_defs.insert(def.as_str().to_owned()); - } - } - - effective_defs + Ok(PreprocessorMetaData { name, imports: used_imports.into_values().collect(), defines, effective_defs }) } } @@ -1044,9 +987,9 @@ defined "; let processor = Preprocessor::default(); - let (_, defines) = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); - println!("defines: {:?}", defines); - let result = processor.preprocess(&WGSL, &defines, true).unwrap(); + let PreprocessorMetaData { defines: shader_defs, .. } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); + println!("defines: {:?}", shader_defs); + let result = processor.preprocess(&WGSL, &shader_defs, true).unwrap(); assert_eq!(result.preprocessed_source, EXPECTED); } @@ -1084,9 +1027,9 @@ bool: false "; let processor = Preprocessor::default(); - let (_, defines) = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); - println!("defines: {:?}", defines); - let result = processor.preprocess(&WGSL, &defines, true).unwrap(); + let PreprocessorMetaData { defines: shader_defs, .. } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); + println!("defines: {:?}", shader_defs); + let result = processor.preprocess(&WGSL, &shader_defs, true).unwrap(); assert_eq!(result.preprocessed_source, EXPECTED); } From c5865ff2b9a4f0c00eaaa3b27a0df09bf6d60e48 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 22:49:13 +0100 Subject: [PATCH 02/30] fix tests --- src/compose/ident_identifier.rs | 144 +++++++++--------- src/compose/mod.rs | 68 +++++---- src/compose/preprocess.rs | 70 +++++---- src/compose/test.rs | 112 ++++++++++---- .../tests/call_entrypoint/include.wgsl | 2 +- src/compose/tests/dup_struct_import/a.wgsl | 6 +- src/compose/tests/dup_struct_import/b.wgsl | 6 +- src/compose/tests/dup_struct_import/top.wgsl | 2 +- src/compose/tests/error_test/include.wgsl | 5 +- .../tests/error_test/wgsl_parse_wrap.wgsl | 6 +- .../tests/error_test/wgsl_valid_wrap.wgsl | 6 +- .../tests/expected/additional_import.txt | 8 +- .../tests/expected/bad_identifiers.txt | 41 +++++ src/compose/tests/expected/big_shaderdefs.txt | 4 +- .../tests/expected/conditional_import_a.txt | 4 +- .../tests/expected/conditional_import_b.txt | 4 +- src/compose/tests/expected/dup_import.txt | 24 ++- .../tests/expected/dup_struct_import.txt | 38 ++--- .../tests/expected/err_validation_2.txt | 4 +- src/compose/tests/expected/glsl_call_wgsl.txt | 4 +- .../tests/expected/glsl_const_import.txt | 8 +- .../tests/expected/glsl_wgsl_const_import.txt | 8 +- src/compose/tests/expected/import_in_decl.txt | 8 +- .../tests/expected/invalid_override_base.txt | 4 +- .../tests/expected/item_import_test.txt | 10 +- src/compose/tests/expected/item_sub_point.txt | 10 +- src/compose/tests/expected/missing_import.txt | 12 +- src/compose/tests/expected/simple_compose.txt | 4 +- .../tests/expected/wgsl_call_entrypoint.txt | 10 +- .../tests/expected/wgsl_glsl_const_import.txt | 4 +- .../invalid_identifiers/top_invalid.wgsl | 23 +++ .../tests/invalid_identifiers/top_valid.wgsl | 22 +++ .../tests/rusty_imports/mod_a_b_c.wgsl | 7 + src/compose/tests/rusty_imports/mod_a_x.wgsl | 5 + src/compose/tests/rusty_imports/top.wgsl | 8 + 35 files changed, 435 insertions(+), 266 deletions(-) create mode 100644 src/compose/tests/expected/bad_identifiers.txt create mode 100644 src/compose/tests/invalid_identifiers/top_invalid.wgsl create mode 100644 src/compose/tests/invalid_identifiers/top_valid.wgsl create mode 100644 src/compose/tests/rusty_imports/mod_a_b_c.wgsl create mode 100644 src/compose/tests/rusty_imports/mod_a_x.wgsl create mode 100644 src/compose/tests/rusty_imports/top.wgsl diff --git a/src/compose/ident_identifier.rs b/src/compose/ident_identifier.rs index 1e0308c..cb80c86 100644 --- a/src/compose/ident_identifier.rs +++ b/src/compose/ident_identifier.rs @@ -1,6 +1,6 @@ use std::collections::{VecDeque, HashMap}; -use super::{ImportDefWithOffset, ImportDefinition}; +use super::{ImportDefWithOffset, ImportDefinition, Composer}; #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] pub enum Token<'a> { @@ -44,7 +44,8 @@ impl<'a> Tokenizer<'a> { let mut current_token_start = 0; let mut current_token = None; - for (ix, char) in src.chars().enumerate() { + // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue + for (ix, char) in src.char_indices() { if let Some(tok) = current_token { match tok { TokenKind::Identifier => { @@ -102,7 +103,7 @@ impl<'a> Iterator for Tokenizer<'a> { } } -pub fn parse_imports(input: &str) -> Result, (&str, usize)> { +pub fn parse_imports<'a>(input: &'a str, declared_imports: &mut HashMap>) -> Result<(), (&'a str, usize)> { let mut tokens = Tokenizer::new(input, false).peekable(); match tokens.next() { @@ -116,7 +117,6 @@ pub fn parse_imports(input: &str) -> Result, (&str, usiz None => return Err(("expected `#import`", input.len())), }; - let mut output = HashMap::default(); let mut stack = Vec::default(); let mut current = String::default(); let mut as_name = None; @@ -160,7 +160,7 @@ pub fn parse_imports(input: &str) -> Result, (&str, usiz None => { if !current.is_empty() { let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| current.rsplit_once("::").map(|(_, name)| name.to_owned()).unwrap_or(current.clone())); - output.insert(used_name, format!("{}{}", stack.join(""), current)); + declared_imports.entry(used_name).or_default().push(format!("{}{}", stack.join(""), current)); current = String::default(); as_name = None; } @@ -186,18 +186,10 @@ pub fn parse_imports(input: &str) -> Result, (&str, usiz return Err(("missing close brace", input.len())); } - Ok(output) -} - -const DECORATION_PRE: &str = "_naga_oil_mod_"; -const DECORATION_POST: &str = "X"; // just to ensure we don't end with a number - -fn decorate(module: &str, item: &str) -> String { - let encoded = data_encoding::BASE32_NOPAD.encode(module.as_bytes()); - format!("{item}{DECORATION_PRE}{encoded}{DECORATION_POST}") + Ok(()) } -pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &HashMap, used_imports: &mut HashMap) -> String { +pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &HashMap>, used_imports: &mut HashMap, allow_ambiguous: bool) -> Result { let tokens = Tokenizer::new(input, true); let mut output = String::with_capacity(input.len()); let mut in_substitution_position = true; @@ -207,20 +199,29 @@ pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &Has Token::Identifier(ident, token_pos) => { if in_substitution_position { let (first, residual) = ident.split_once("::").unwrap_or((ident, "")); - let mut full_path = declared_imports.get(first).cloned().unwrap_or(first.to_owned()); - if !residual.is_empty() { - full_path.push_str("::"); - full_path.push_str(residual); - } + let full_paths = declared_imports.get(first).cloned().unwrap_or(vec![first.to_owned()]); - if let Some((module, item)) = full_path.rsplit_once("::") { - used_imports.entry(module.to_owned()).or_insert_with(|| { - ImportDefWithOffset { definition: ImportDefinition { import: module.to_owned(), ..Default::default() }, offset: offset + token_pos } - }).definition.items.push(item.to_owned()); - output.push_str(&decorate(module, item)) - } else { - output.push_str(&full_path); + if !allow_ambiguous && full_paths.len() > 1 { + return Err(offset + token_pos); } + + for mut full_path in full_paths { + if !residual.is_empty() { + full_path.push_str("::"); + full_path.push_str(residual); + } + + if let Some((module, item)) = full_path.rsplit_once("::") { + used_imports.entry(module.to_owned()).or_insert_with(|| { + println!("setting import offset for {module} to {offset}+{token_pos} = {}", offset + token_pos); + ImportDefWithOffset { definition: ImportDefinition { import: module.to_owned(), ..Default::default() }, offset: offset + token_pos } + }).definition.items.push(item.to_owned()); + output.push_str(item); + output.push_str(&Composer::decorate(module)); + } else { + output.push_str(&full_path); + } + } } else { output.push_str(ident); } @@ -238,82 +239,89 @@ pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &Has in_substitution_position = true; } - output + Ok(output) +} + +#[cfg(test)] +fn test_parse(input: &str) -> Result>, (&str, usize)> { + let mut declared_imports = HashMap::default(); + parse_imports(input, &mut declared_imports)?; + Ok(declared_imports) } - + #[test] fn import_tokens() { let input = r" #import a::b "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([("b".to_owned(), "a::b".to_owned())]))); + assert_eq!(test_parse(input), Ok(HashMap::from_iter([("b".to_owned(), vec!("a::b".to_owned()))]))); let input = r" #import a::{b, c} "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("b".to_owned(), "a::b".to_owned()), - ("c".to_owned(), "a::c".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("b".to_owned(), vec!("a::b".to_owned())), + ("c".to_owned(), vec!("a::c".to_owned())), ]))); let input = r" #import a::{b as d, c} "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("d".to_owned(), "a::b".to_owned()), - ("c".to_owned(), "a::c".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b".to_owned())), + ("c".to_owned(), vec!("a::c".to_owned())), ]))); let input = r" #import a::{b::{c, d}, e} "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("c".to_owned(), "a::b::c".to_owned()), - ("d".to_owned(), "a::b::d".to_owned()), - ("e".to_owned(), "a::e".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), + ("e".to_owned(), vec!("a::e".to_owned())), ]))); let input = r" #import a::b::{c, d}, e "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("c".to_owned(), "a::b::c".to_owned()), - ("d".to_owned(), "a::b::d".to_owned()), - ("e".to_owned(), "e".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), + ("e".to_owned(), vec!("e".to_owned())), ]))); let input = r" #import a, b "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("a".to_owned(), "a".to_owned()), - ("b".to_owned(), "b".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("a".to_owned(), vec!("a".to_owned())), + ("b".to_owned(), vec!("b".to_owned())), ]))); let input = r" #import a::b c, d "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("c".to_owned(), "a::b::c".to_owned()), - ("d".to_owned(), "a::b::d".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), ]))); let input = r" #import a::b c "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("c".to_owned(), "a::b::c".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), ]))); let input = r" #import a::b::{c::{d, e}, f, g::{h as i, j}} "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("d".to_owned(), "a::b::c::d".to_owned()), - ("e".to_owned(), "a::b::c::e".to_owned()), - ("f".to_owned(), "a::b::f".to_owned()), - ("i".to_owned(), "a::b::g::h".to_owned()), - ("j".to_owned(), "a::b::g::j".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b::c::d".to_owned())), + ("e".to_owned(), vec!("a::b::c::e".to_owned())), + ("f".to_owned(), vec!("a::b::f".to_owned())), + ("i".to_owned(), vec!("a::b::g::h".to_owned())), + ("j".to_owned(), vec!("a::b::g::j".to_owned())), ]))); let input = r" @@ -326,31 +334,31 @@ fn import_tokens() { } } "; - assert_eq!(parse_imports(input), Ok(HashMap::from_iter([ - ("d".to_owned(), "a::b::c::d".to_owned()), - ("e".to_owned(), "a::b::c::e".to_owned()), - ("f".to_owned(), "a::b::f".to_owned()), - ("i".to_owned(), "a::b::g::h".to_owned()), - ("m".to_owned(), "a::b::g::j::k::l".to_owned()), + assert_eq!(test_parse(input), Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b::c::d".to_owned())), + ("e".to_owned(), vec!("a::b::c::e".to_owned())), + ("f".to_owned(), vec!("a::b::f".to_owned())), + ("i".to_owned(), vec!("a::b::g::h".to_owned())), + ("m".to_owned(), vec!("a::b::g::j::k::l".to_owned())), ]))); let input = r" #import a::b::{ "; - assert!(parse_imports(input).is_err()); + assert!(test_parse(input).is_err()); let input = r" #import a::b::{c}} "; - assert!(parse_imports(input).is_err()); + assert!(test_parse(input).is_err()); let input = r" #import a::b::{c}} "; - assert!(parse_imports(input).is_err()); + assert!(test_parse(input).is_err()); let input = r" #import a::b{{c,d}} "; - assert!(parse_imports(input).is_err()); + assert!(test_parse(input).is_err()); } diff --git a/src/compose/mod.rs b/src/compose/mod.rs index d62bedc..3a034e9 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -339,7 +339,7 @@ impl Default for Composer { check_decoration_regex: Regex::new(format!("({}|{})", regex_syntax::escape(DECORATION_PRE), regex_syntax::escape(DECORATION_OVERRIDE_PRE)).as_str()).unwrap(), undecorate_regex: Regex::new( format!( - "{}([A-Z0-9]*){}", + r"([\w\d_]+){}([A-Z0-9]*){}", regex_syntax::escape(DECORATION_PRE), regex_syntax::escape(DECORATION_POST) ) @@ -349,7 +349,7 @@ impl Default for Composer { virtual_fn_regex: Regex::new(r"(?P[\s]*virtual\s+fn\s+)(?P[^\s]+)(?P\s*)\(").unwrap(), override_fn_regex: Regex::new( format!( - r"(?P[\s]*override\s+fn\s*){}(?P[^\s]+){}(?P[^\s]+)(?P\s*)\(", + r"(override\s+fn\s+)([^\s]+){}([\w\d]+){}(\s*)\(", regex_syntax::escape(DECORATION_PRE), regex_syntax::escape(DECORATION_POST) ) @@ -370,11 +370,11 @@ impl Default for Composer { } } -const DECORATION_PRE: &str = "_naga_oil_mod_"; -const DECORATION_POST: &str = "_member"; +const DECORATION_PRE: &str = "X_naga_oil_mod_X"; +const DECORATION_POST: &str = "X"; // must be same length as DECORATION_PRE for spans to work -const DECORATION_OVERRIDE_PRE: &str = "_naga_oil_vrt_"; +const DECORATION_OVERRIDE_PRE: &str = "X_naga_oil_vrt_X"; struct IrBuildResult { module: naga::Module, @@ -385,16 +385,16 @@ struct IrBuildResult { impl Composer { pub fn decorated_name(module_name: Option<&str>, item_name: &str) -> String { match module_name { - Some(module_name) => format!("{}{}", Self::decorate(module_name), item_name), + Some(module_name) => format!("{}{}", item_name, Self::decorate(module_name)), None => item_name.to_owned(), } } - fn decorate(as_name: &str) -> String { - let as_name = data_encoding::BASE32_NOPAD.encode(as_name.as_bytes()); - format!("{DECORATION_PRE}{as_name}{DECORATION_POST}") + fn decorate(module: &str) -> String { + let encoded = data_encoding::BASE32_NOPAD.encode(module.as_bytes()); + format!("{DECORATION_PRE}{encoded}{DECORATION_POST}") } - + fn decode(from: &str) -> String { String::from_utf8(data_encoding::BASE32_NOPAD.decode(from.as_bytes()).unwrap()).unwrap() } @@ -403,7 +403,7 @@ impl Composer { let undecor = self .undecorate_regex .replace_all(string, |caps: ®ex::Captures| { - format!("{}::", Self::decode(caps.get(1).unwrap().as_str())) + format!("{}::{}", Self::decode(caps.get(2).unwrap().as_str()), caps.get(1).unwrap().as_str()) }); let undecor = @@ -703,7 +703,7 @@ impl Composer { .collect(); for (h, ty) in source_ir.types.iter() { if let Some(name) = &ty.name { - let decorated_type_name = format!("{module_decoration}{name}"); + let decorated_type_name = format!("{name}{module_decoration}"); if !owned_types.contains(&decorated_type_name) { continue; } @@ -740,11 +740,11 @@ impl Composer { .constants .iter() .flat_map(|(_, c)| c.name.as_deref()) - .filter(|name| name.starts_with(module_decoration)) + .filter(|name| name.ends_with(module_decoration)) .collect(); for (h, c) in source_ir.constants.iter() { if let Some(name) = &c.name { - if name.starts_with(module_decoration) && !recompiled_consts.contains(name.as_str()) + if name.ends_with(module_decoration) && !recompiled_consts.contains(name.as_str()) { return Err(ComposerErrorInner::InvalidIdentifier { original: name.clone(), @@ -758,11 +758,11 @@ impl Composer { .global_variables .iter() .flat_map(|(_, c)| c.name.as_deref()) - .filter(|name| name.starts_with(module_decoration)) + .filter(|name| name.ends_with(module_decoration)) .collect(); for (h, gv) in source_ir.global_variables.iter() { if let Some(name) = &gv.name { - if name.starts_with(module_decoration) + if name.ends_with(module_decoration) && !recompiled_globals.contains(name.as_str()) { return Err(ComposerErrorInner::InvalidIdentifier { @@ -777,11 +777,11 @@ impl Composer { .functions .iter() .flat_map(|(_, c)| c.name.as_deref()) - .filter(|name| name.starts_with(module_decoration)) + .filter(|name| name.ends_with(module_decoration)) .collect(); for (h, f) in source_ir.functions.iter() { if let Some(name) = &f.name { - if name.starts_with(module_decoration) && !recompiled_fns.contains(name.as_str()) { + if name.ends_with(module_decoration) && !recompiled_fns.contains(name.as_str()) { return Err(ComposerErrorInner::InvalidIdentifier { original: name.clone(), at: source_ir.functions.get_span(h), @@ -868,8 +868,8 @@ impl Composer { let source = self.override_fn_regex .replace_all(&source, |cap: ®ex::Captures| { - let target_module = cap.get(2).unwrap().as_str().to_owned(); - let target_function = cap.get(3).unwrap().as_str().to_owned(); + let target_module = cap.get(3).unwrap().as_str().to_owned(); + let target_function = cap.get(2).unwrap().as_str().to_owned(); #[cfg(not(feature = "override_any"))] { @@ -879,7 +879,8 @@ impl Composer { match module_set { None => { - let pos = cap.get(2).unwrap().start(); + // TODO this should be unreachable? + let pos = cap.get(3).unwrap().start(); override_error = Some(wrap_err( ComposerErrorInner::ImportNotFound(raw_module_name, pos), )); @@ -887,7 +888,7 @@ impl Composer { Some(module_set) => { let module = module_set.get_module(shader_defs).unwrap(); if !module.virtual_functions.contains(&target_function) { - let pos = cap.get(3).unwrap().start(); + let pos = cap.get(2).unwrap().start(); override_error = Some(wrap_err(ComposerErrorInner::OverrideNotVirtual { name: target_function.clone(), @@ -900,17 +901,17 @@ impl Composer { let base_name = format!( "{}{}{}{}", + target_function.as_str(), DECORATION_PRE, target_module.as_str(), DECORATION_POST, - target_function.as_str() ); let rename = format!( "{}{}{}{}", + target_function.as_str(), DECORATION_OVERRIDE_PRE, target_module.as_str(), DECORATION_POST, - target_function.as_str() ); let replacement_str = format!( @@ -967,7 +968,7 @@ impl Composer { override_functions .entry(base_name.clone()) .or_default() - .push(format!("{module_decoration}{rename}")); + .push(format!("{rename}{module_decoration}")); } // rename and record owned items (except types which can't be mutably accessed) @@ -975,7 +976,7 @@ impl Composer { for (h, c) in source_ir.constants.iter_mut() { if let Some(name) = c.name.as_mut() { if !name.contains(DECORATION_PRE) { - *name = format!("{module_decoration}{name}"); + *name = format!("{name}{module_decoration}"); owned_constants.insert(name.clone(), h); } } @@ -985,7 +986,7 @@ impl Composer { for (h, gv) in source_ir.global_variables.iter_mut() { if let Some(name) = gv.name.as_mut() { if !name.contains(DECORATION_PRE) { - *name = format!("{module_decoration}{name}"); + *name = format!("{name}{module_decoration}"); owned_vars.insert(name.clone(), h); } @@ -996,7 +997,7 @@ impl Composer { for (h_f, f) in source_ir.functions.iter_mut() { if let Some(name) = f.name.as_mut() { if !name.contains(DECORATION_PRE) { - *name = format!("{module_decoration}{name}"); + *name = format!("{name}{module_decoration}"); // create dummy header function let header_function = naga::Function { @@ -1020,8 +1021,8 @@ impl Composer { for ep in &mut source_ir.entry_points { ep.function.name = Some(format!( "{}{}", + ep.function.name.as_deref().unwrap_or("main"), module_decoration, - ep.function.name.as_deref().unwrap_or("main") )); let header_function = naga::Function { name: ep.function.name.clone(), @@ -1059,7 +1060,7 @@ impl Composer { for (h, ty) in source_ir.types.iter() { if let Some(name) = &ty.name { if !name.contains(DECORATION_PRE) { - let name = format!("{module_decoration}{name}"); + let name = format!("{name}{module_decoration}"); owned_types.insert(name.clone()); // copy and rename types module_builder.rename_type(&h, Some(name.clone())); @@ -1166,7 +1167,7 @@ impl Composer { let items: Option> = items.map(|items| { items .iter() - .map(|item| format!("{}{}", composable.decorated_name, item)) + .map(|item| format!("{}{}", item, composable.decorated_name)) .collect() }); let items = items.as_ref(); @@ -1211,7 +1212,10 @@ impl Composer { for (h_f, f) in source_ir.functions.iter() { if let Some(name) = &f.name { if composable.owned_functions.contains(name) - && items.map_or(true, |items| items.contains(name)) + && ( + items.map_or(true, |items| items.contains(name)) || + composable.override_functions.values().any(|v| v.contains(name)) + ) { let span = composable.module_ir.functions.get_span(h_f); derived.import_function_if_new(f, span); diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index cd88936..8bb9223 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -288,32 +288,40 @@ impl Preprocessor { line = lines.next().unwrap().0; } - declared_imports.extend( - parse_imports(import_lines.as_str()).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))? - ); + parse_imports(import_lines.as_str(), &mut declared_imports).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))?; output = true; } else { - let mut line_with_defs = original_line.to_string(); - for capture in self.def_regex.captures_iter(&line) { - let def = capture.get(1).unwrap(); - if let Some(def) = shader_defs.get(def.as_str()) { - line_with_defs = self - .def_regex - .replace(&line_with_defs, def.value_as_string()) - .to_string(); + let replaced_lines = [original_line, &line].map(|input| { + let mut output = input.to_string(); + for capture in self.def_regex.captures_iter(&input) { + let def = capture.get(1).unwrap(); + if let Some(def) = shader_defs.get(def.as_str()) { + output = self + .def_regex + .replace(&output, def.value_as_string()) + .to_string(); + } } - } - for capture in self.def_regex_delimited.captures_iter(&line) { - let def = capture.get(1).unwrap(); - if let Some(def) = shader_defs.get(def.as_str()) { - line_with_defs = self - .def_regex_delimited - .replace(&line_with_defs, def.value_as_string()) - .to_string(); + for capture in self.def_regex_delimited.captures_iter(&input) { + let def = capture.get(1).unwrap(); + if let Some(def) = shader_defs.get(def.as_str()) { + output = self + .def_regex_delimited + .replace(&output, def.value_as_string()) + .to_string(); + } } - } + output + }); + + let original_line = &replaced_lines[0]; + let decommented_line = &replaced_lines[1]; - let item_replaced_line = substitute_identifiers(line_with_defs.as_str(), offset, &declared_imports, &mut used_imports); + // we don't want to capture imports from comments so we run using a dummy used_imports, and disregard any errors + let item_replaced_line = substitute_identifiers(original_line, offset, &declared_imports, &mut Default::default(), true).unwrap(); + // we also run against the de-commented line to replace real imports, and throw an error if appropriate + let _ = substitute_identifiers(decommented_line, offset, &declared_imports, &mut used_imports, false) + .map_err(|pos| ComposerErrorInner::ImportParseError("Ambiguous import path for item".to_owned(), pos))?; final_string.push_str(&item_replaced_line); let diff = line.len().saturating_sub(item_replaced_line.len()); @@ -364,16 +372,17 @@ impl Preprocessor { let mut lines = lines.replace_comments().peekable(); while let Some(mut line) = lines.next() { - if let Some(def) = self.check_scope(&HashMap::default(), &line, None, offset)?.1 { - effective_defs.insert(def.to_owned()); + let (is_scope, def) = self.check_scope(&HashMap::default(), &line, None, offset)?; + + if is_scope { + if let Some(def) = def { + effective_defs.insert(def.to_owned()); + } } else if self.import_regex.is_match(&line) { let mut import_lines = String::default(); let mut open_count = 0; loop { - // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) - offset += line.len() + 1; - open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); @@ -383,12 +392,13 @@ impl Preprocessor { break; } + // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) + offset += line.len() + 1; + line = lines.next().unwrap(); } - declared_imports.extend( - parse_imports(import_lines.as_str()).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))? - ); + parse_imports(import_lines.as_str(), &mut declared_imports).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))?; } else if let Some(cap) = self.define_import_path_regex.captures(&line) { name = Some(cap.get(1).unwrap().as_str().to_string()); } else if let Some(cap) = self.define_shader_def_regex.captures(&line) { @@ -415,7 +425,7 @@ impl Preprocessor { return Err(ComposerErrorInner::DefineInModule(offset)); } } else { - substitute_identifiers(&line, offset, &declared_imports, &mut used_imports); + substitute_identifiers(&line, offset, &declared_imports, &mut used_imports, true).unwrap(); } offset += line.len() + 1; diff --git a/src/compose/test.rs b/src/compose/test.rs index f64c47d..b6b05fb 100644 --- a/src/compose/test.rs +++ b/src/compose/test.rs @@ -12,7 +12,7 @@ mod test { }; use crate::compose::{ - ComposableModuleDescriptor, Composer, ComposerErrorInner, ImportDefinition, + ComposableModuleDescriptor, Composer, ImportDefinition, NagaModuleDescriptor, ShaderDefValue, ShaderLanguage, ShaderType, }; @@ -156,6 +156,9 @@ mod test { naga::back::wgsl::WriterFlags::EXPLICIT_TYPES, ) .unwrap(); + let mut wgsl: Vec<_> = wgsl.lines().collect(); + wgsl.sort(); + let wgsl = wgsl.join("\n"); // println!("{}", wgsl); // let mut f = std::fs::File::create("dup_import.txt").unwrap(); @@ -510,7 +513,18 @@ mod test { }) .unwrap(); - assert_eq!(test_shader(&mut composer), 3.0); + // this test doesn't work any more. + // overrides only work if the composer realises the module is required. + // not we can't just blindly import any `#import`ed items because that would break: + // #import a::b + // a::b::c::d(); + // the path would be interpreted as a module when it may actually + // be only a fragment of a path to a module. + // so either i need to add another directive (#import_overrides) + // or we just limit overrides to modules included via the additional_modules + // in `Composer::make_naga_module` and `Composer::add_composable_module` + + // assert_eq!(test_shader(&mut composer), 3.0); } #[cfg(feature = "test_shader")] @@ -848,23 +862,6 @@ mod test { fn bad_identifiers() { let mut composer = Composer::default(); - let check_err = |composer: &mut Composer, name: &str| -> bool { - let result = composer.make_naga_module(NagaModuleDescriptor { - source: &format!("#import {name}"), - file_path: name, - ..Default::default() - }); - - if let Err(err) = &result { - if let ComposerErrorInner::InvalidIdentifier { original, .. } = &err.inner { - return original.ends_with("bad_"); - } - } - - println!("{result:?}"); - false - }; - composer .add_composable_module(ComposableModuleDescriptor { source: include_str!("tests/invalid_identifiers/const.wgsl"), @@ -872,8 +869,6 @@ mod test { ..Default::default() }) .unwrap(); - assert!(check_err(&mut composer, "consts")); - composer .add_composable_module(ComposableModuleDescriptor { source: include_str!("tests/invalid_identifiers/fn.wgsl"), @@ -881,8 +876,6 @@ mod test { ..Default::default() }) .unwrap(); - assert!(check_err(&mut composer, "fns")); - composer .add_composable_module(ComposableModuleDescriptor { source: include_str!("tests/invalid_identifiers/global.wgsl"), @@ -890,8 +883,6 @@ mod test { ..Default::default() }) .unwrap(); - assert!(check_err(&mut composer, "globals")); - composer .add_composable_module(ComposableModuleDescriptor { source: include_str!("tests/invalid_identifiers/struct_member.wgsl"), @@ -899,8 +890,6 @@ mod test { ..Default::default() }) .unwrap(); - assert!(check_err(&mut composer, "struct_members")); - composer .add_composable_module(ComposableModuleDescriptor { source: include_str!("tests/invalid_identifiers/struct.wgsl"), @@ -908,7 +897,39 @@ mod test { ..Default::default() }) .unwrap(); - assert!(check_err(&mut composer, "structs")); + let module = composer.make_naga_module(NagaModuleDescriptor { + source: include_str!("tests/invalid_identifiers/top_valid.wgsl"), + file_path: "tests/invalid_identifiers/top_valid.wgsl", + ..Default::default() + }).unwrap(); + + let info = naga::valid::Validator::new( + naga::valid::ValidationFlags::all(), + naga::valid::Capabilities::default(), + ) + .validate(&module) + .unwrap(); + let wgsl = naga::back::wgsl::write_string( + &module, + &info, + naga::back::wgsl::WriterFlags::EXPLICIT_TYPES, + ) + .unwrap(); + let mut wgsl: Vec<_> = wgsl.lines().collect(); + wgsl.sort(); + let wgsl = wgsl.join("\n"); + + // let mut f = std::fs::File::create("bad_identifiers.txt").unwrap(); + // f.write_all(wgsl.as_bytes()).unwrap(); + // drop(f); + + output_eq!(wgsl, "tests/expected/bad_identifiers.txt"); + + composer.make_naga_module(NagaModuleDescriptor { + source: include_str!("tests/invalid_identifiers/top_invalid.wgsl"), + file_path: "tests/invalid_identifiers/top_invalid.wgsl", + ..Default::default() + }).err().unwrap(); } #[test] @@ -960,6 +981,9 @@ mod test { naga::back::wgsl::WriterFlags::EXPLICIT_TYPES, ) .unwrap(); + let mut wgsl: Vec<_> = wgsl.lines().collect(); + wgsl.sort(); + let wgsl = wgsl.join("\n"); // let mut f = std::fs::File::create("dup_struct_import.txt").unwrap(); // f.write_all(wgsl.as_bytes()).unwrap(); @@ -1083,6 +1107,38 @@ mod test { output_eq!(wgsl, "tests/expected/conditional_import_b.txt"); } + #[cfg(feature = "test_shader")] + #[test] + fn rusty_imports() { + let mut composer = Composer::default(); + + composer + .add_composable_module(ComposableModuleDescriptor { + source: include_str!("tests/rusty_imports/mod_a_b_c.wgsl"), + file_path: "tests/rusty_imports/mod_a_b_c.wgsl", + ..Default::default() + }) + .unwrap(); + + composer + .add_composable_module(ComposableModuleDescriptor { + source: include_str!("tests/rusty_imports/mod_a_x.wgsl"), + file_path: "tests/rusty_imports/mod_a_x.wgsl", + ..Default::default() + }) + .unwrap(); + + composer + .add_composable_module(ComposableModuleDescriptor { + source: include_str!("tests/rusty_imports/top.wgsl"), + file_path: "tests/rusty_imports/top.wgsl", + ..Default::default() + }) + .unwrap(); + + assert_eq!(test_shader(&mut composer), 36.0); + } + // actually run a shader and extract the result // needs the composer to contain a module called "test_module", with a function called "entry_point" returning an f32. fn test_shader(composer: &mut Composer) -> f32 { diff --git a/src/compose/tests/call_entrypoint/include.wgsl b/src/compose/tests/call_entrypoint/include.wgsl index ccf932b..c44c757 100644 --- a/src/compose/tests/call_entrypoint/include.wgsl +++ b/src/compose/tests/call_entrypoint/include.wgsl @@ -8,5 +8,5 @@ fn non_ep(f: f32) -> f32 { fn fragment( @builtin(position) frag_coord: vec4, ) -> @location(0) vec4 { - return vec4(1.0); + return vec4(1.5 * frag_coord); } \ No newline at end of file diff --git a/src/compose/tests/dup_struct_import/a.wgsl b/src/compose/tests/dup_struct_import/a.wgsl index e0b484d..cba7a1a 100644 --- a/src/compose/tests/dup_struct_import/a.wgsl +++ b/src/compose/tests/dup_struct_import/a.wgsl @@ -2,7 +2,7 @@ #import struct fn a() -> struct::MyStruct { - var s: struct::MyStruct; - s.value = 1.0; - return s; + var s_a: struct::MyStruct; + s_a.value = 1.0; + return s_a; } \ No newline at end of file diff --git a/src/compose/tests/dup_struct_import/b.wgsl b/src/compose/tests/dup_struct_import/b.wgsl index b8d5630..aad40b1 100644 --- a/src/compose/tests/dup_struct_import/b.wgsl +++ b/src/compose/tests/dup_struct_import/b.wgsl @@ -2,7 +2,7 @@ #import struct fn b() -> struct::MyStruct { - var s: struct::MyStruct; - s.value = 2.0; - return s; + var s_b: struct::MyStruct; + s_b.value = 2.0; + return s_b; } \ No newline at end of file diff --git a/src/compose/tests/dup_struct_import/top.wgsl b/src/compose/tests/dup_struct_import/top.wgsl index a4a835e..a5fa48f 100644 --- a/src/compose/tests/dup_struct_import/top.wgsl +++ b/src/compose/tests/dup_struct_import/top.wgsl @@ -4,5 +4,5 @@ fn main() -> f32 { let a = a::a(); let b = b::b(); - return a.value + b.value; + return a.value / b.value; } \ No newline at end of file diff --git a/src/compose/tests/error_test/include.wgsl b/src/compose/tests/error_test/include.wgsl index 056cdd4..406ba51 100644 --- a/src/compose/tests/error_test/include.wgsl +++ b/src/compose/tests/error_test/include.wgsl @@ -6,4 +6,7 @@ or here, just moving lines around a bit #import missing -fn sub() {} +fn sub() { + // have to use something for it to be declared missing + let x = missing::y(); +} diff --git a/src/compose/tests/error_test/wgsl_parse_wrap.wgsl b/src/compose/tests/error_test/wgsl_parse_wrap.wgsl index 5f8f861..aecc9d7 100644 --- a/src/compose/tests/error_test/wgsl_parse_wrap.wgsl +++ b/src/compose/tests/error_test/wgsl_parse_wrap.wgsl @@ -1,3 +1,3 @@ -#import wgsl_parse_err - -fn ok() {} \ No newline at end of file +fn ok() { + wgsl_parse_err::woops(); +} \ No newline at end of file diff --git a/src/compose/tests/error_test/wgsl_valid_wrap.wgsl b/src/compose/tests/error_test/wgsl_valid_wrap.wgsl index 60e25ef..5260f9f 100644 --- a/src/compose/tests/error_test/wgsl_valid_wrap.wgsl +++ b/src/compose/tests/error_test/wgsl_valid_wrap.wgsl @@ -1,3 +1,3 @@ -#import valid_inc - -fn whatever() {} \ No newline at end of file +fn whatever() { + valid_inc::main(); +} \ No newline at end of file diff --git a/src/compose/tests/expected/additional_import.txt b/src/compose/tests/expected/additional_import.txt index 59f8b91..f57a22d 100644 --- a/src/compose/tests/expected/additional_import.txt +++ b/src/compose/tests/expected/additional_import.txt @@ -1,14 +1,14 @@ -fn _naga_oil_mod_N53GK4TSNFSGCYTMMU_memberfunc() -> f32 { +fn funcX_naga_oil_mod_XN53GK4TSNFSGCYTMMUX() -> f32 { return 1.0; } -fn _naga_oil_mod_OBWHKZ3JNY_member_naga_oil_vrt_N53GK4TSNFSGCYTMMU_memberfunc() -> f32 { - let _e0: f32 = _naga_oil_mod_N53GK4TSNFSGCYTMMU_memberfunc(); +fn funcX_naga_oil_vrt_XN53GK4TSNFSGCYTMMUXX_naga_oil_mod_XOBWHKZ3JNYX() -> f32 { + let _e0: f32 = funcX_naga_oil_mod_XN53GK4TSNFSGCYTMMUX(); return (_e0 + 1.0); } fn entry_point() -> f32 { - let _e0: f32 = _naga_oil_mod_OBWHKZ3JNY_member_naga_oil_vrt_N53GK4TSNFSGCYTMMU_memberfunc(); + let _e0: f32 = funcX_naga_oil_vrt_XN53GK4TSNFSGCYTMMUXX_naga_oil_mod_XOBWHKZ3JNYX(); return _e0; } diff --git a/src/compose/tests/expected/bad_identifiers.txt b/src/compose/tests/expected/bad_identifiers.txt new file mode 100644 index 0000000..164f345 --- /dev/null +++ b/src/compose/tests/expected/bad_identifiers.txt @@ -0,0 +1,41 @@ + + + + + + + + + + d.fine = 3.0; + e.fine_member = 4.0; + fine: f32, + fine_member: f32, + let _e11: f32 = bad_X_naga_oil_mod_XM5WG6YTBNRZQX; + let _e22: f32 = d.fine; + let _e25: f32 = e.fine_member; + let _e4: f32 = fineX_naga_oil_mod_XMZXHGX(1.0); + let _e6: f32 = bad_X_naga_oil_mod_XMZXHGX(2.0); + let _e9: f32 = fineX_naga_oil_mod_XM5WG6YTBNRZQX; + let a: f32 = (fineX_naga_oil_mod_XMNXW443UOMX + bad_X_naga_oil_mod_XMNXW443UOMX); + let b: f32 = (_e4 + _e6); + let c: f32 = (_e9 + _e11); + return ((((a + b) + c) + _e22) + _e25); + return in; + return in_1; + var d: IsFineX_naga_oil_mod_XON2HE5LDORZQX; + var e: Isbad_X_naga_oil_mod_XON2HE5LDORZQX; +const bad_X_naga_oil_mod_XMNXW443UOMX: f32 = 1.0; +const fineX_naga_oil_mod_XMNXW443UOMX: f32 = 1.0; +fn bad_X_naga_oil_mod_XMZXHGX(in_1: f32) -> f32 { +fn fineX_naga_oil_mod_XMZXHGX(in: f32) -> f32 { +fn main() -> f32 { +struct IsFineX_naga_oil_mod_XON2HE5LDORZQX { +struct Isbad_X_naga_oil_mod_XON2HE5LDORZQX { +var bad_X_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1.0; +var fineX_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1.0; +} +} +} +} +} \ No newline at end of file diff --git a/src/compose/tests/expected/big_shaderdefs.txt b/src/compose/tests/expected/big_shaderdefs.txt index ae8b6d6..45535e0 100644 --- a/src/compose/tests/expected/big_shaderdefs.txt +++ b/src/compose/tests/expected/big_shaderdefs.txt @@ -1,4 +1,4 @@ -fn _naga_oil_mod_NVXWI_memberf() -> f32 { +fn fX_naga_oil_mod_XNVXWIX() -> f32 { var x: f32; x = 0.0; @@ -8,7 +8,7 @@ fn _naga_oil_mod_NVXWI_memberf() -> f32 { } fn main() -> f32 { - let _e0: f32 = _naga_oil_mod_NVXWI_memberf(); + let _e0: f32 = fX_naga_oil_mod_XNVXWIX(); return _e0; } diff --git a/src/compose/tests/expected/conditional_import_a.txt b/src/compose/tests/expected/conditional_import_a.txt index 2f41734..6e9f4c1 100644 --- a/src/compose/tests/expected/conditional_import_a.txt +++ b/src/compose/tests/expected/conditional_import_a.txt @@ -1,6 +1,6 @@ -const _naga_oil_mod_ME_memberC: u32 = 1u; +const CX_naga_oil_mod_XMEX: u32 = 1u; fn main() -> u32 { - return _naga_oil_mod_ME_memberC; + return CX_naga_oil_mod_XMEX; } diff --git a/src/compose/tests/expected/conditional_import_b.txt b/src/compose/tests/expected/conditional_import_b.txt index 31a7a56..1284aa5 100644 --- a/src/compose/tests/expected/conditional_import_b.txt +++ b/src/compose/tests/expected/conditional_import_b.txt @@ -1,6 +1,6 @@ -const _naga_oil_mod_MI_memberC: u32 = 2u; +const CX_naga_oil_mod_XMIX: u32 = 2u; fn main() -> u32 { - return _naga_oil_mod_MI_memberC; + return CX_naga_oil_mod_XMIX; } diff --git a/src/compose/tests/expected/dup_import.txt b/src/compose/tests/expected/dup_import.txt index f20ad86..ebc0cae 100644 --- a/src/compose/tests/expected/dup_import.txt +++ b/src/compose/tests/expected/dup_import.txt @@ -1,20 +1,16 @@ -const _naga_oil_mod_MNXW443UOM_memberPI: f32 = 3.0999999046325684; -fn _naga_oil_mod_ME_memberf() -> f32 { - return (_naga_oil_mod_MNXW443UOM_memberPI * 1.0); -} -fn _naga_oil_mod_MI_memberf() -> f32 { - return (_naga_oil_mod_MNXW443UOM_memberPI * 2.0); -} -fn _naga_oil_mod_MI_memberg() -> f32 { - return (_naga_oil_mod_MNXW443UOM_memberPI * 2.0); -} -fn main() -> f32 { - let _e0: f32 = _naga_oil_mod_ME_memberf(); - let _e1: f32 = _naga_oil_mod_MI_memberf(); + let _e0: f32 = fX_naga_oil_mod_XMEX(); + let _e1: f32 = fX_naga_oil_mod_XMIX(); + return (PIX_naga_oil_mod_XMNXW443UOMX * 1.0); + return (PIX_naga_oil_mod_XMNXW443UOMX * 2.0); return (_e0 * _e1); +const PIX_naga_oil_mod_XMNXW443UOMX: f32 = 3.0999999046325684; +fn fX_naga_oil_mod_XMEX() -> f32 { +fn fX_naga_oil_mod_XMIX() -> f32 { +fn main() -> f32 { } - +} +} \ No newline at end of file diff --git a/src/compose/tests/expected/dup_struct_import.txt b/src/compose/tests/expected/dup_struct_import.txt index 910e5b5..e0eed15 100644 --- a/src/compose/tests/expected/dup_struct_import.txt +++ b/src/compose/tests/expected/dup_struct_import.txt @@ -1,26 +1,26 @@ -struct _naga_oil_mod_ON2HE5LDOQ_memberMyStruct { - value: f32, -} -fn _naga_oil_mod_ME_membera() -> _naga_oil_mod_ON2HE5LDOQ_memberMyStruct { - var s: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct; - s.value = 1.0; - let _e3: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct = s; - return _e3; -} -fn _naga_oil_mod_MI_memberb() -> _naga_oil_mod_ON2HE5LDOQ_memberMyStruct { - var s_1: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct; - s_1.value = 2.0; - let _e3: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct = s_1; - return _e3; -} + + let _e0: MyStructX_naga_oil_mod_XON2HE5LDOQX = aX_naga_oil_mod_XMEX(); + let _e1: MyStructX_naga_oil_mod_XON2HE5LDOQX = bX_naga_oil_mod_XMIX(); + let _e3: MyStructX_naga_oil_mod_XON2HE5LDOQX = s_a; + let _e3: MyStructX_naga_oil_mod_XON2HE5LDOQX = s_b; + return (_e0.value / _e1.value); + return _e3; + return _e3; + s_a.value = 1.0; + s_b.value = 2.0; + value: f32, + var s_a: MyStructX_naga_oil_mod_XON2HE5LDOQX; + var s_b: MyStructX_naga_oil_mod_XON2HE5LDOQX; +fn aX_naga_oil_mod_XMEX() -> MyStructX_naga_oil_mod_XON2HE5LDOQX { +fn bX_naga_oil_mod_XMIX() -> MyStructX_naga_oil_mod_XON2HE5LDOQX { fn main() -> f32 { - let _e0: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct = _naga_oil_mod_ME_membera(); - let _e1: _naga_oil_mod_ON2HE5LDOQ_memberMyStruct = _naga_oil_mod_MI_memberb(); - return (_e0.value + _e1.value); +struct MyStructX_naga_oil_mod_XON2HE5LDOQX { } - +} +} +} \ No newline at end of file diff --git a/src/compose/tests/expected/err_validation_2.txt b/src/compose/tests/expected/err_validation_2.txt index e6276ba..9bd41d1 100644 --- a/src/compose/tests/expected/err_validation_2.txt +++ b/src/compose/tests/expected/err_validation_2.txt @@ -1,10 +1,10 @@ -error: failed to build a valid final module: Function [2] 'valid_inc::func' is invalid +error: failed to build a valid final module: Function [1] 'valid_inc::func' is invalid โ”Œโ”€ tests/error_test/wgsl_valid_err.wgsl:7:1 โ”‚ 7 โ”‚ โ•ญ fn func() -> f32 { 8 โ”‚ โ”‚ return 1u; โ”‚ โ”‚ ^^ naga::Expression [1] - โ”‚ โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€^ naga::Function [2] + โ”‚ โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€^ naga::Function [1] โ”‚ = The `return` value Some([1]) does not match the function return value diff --git a/src/compose/tests/expected/glsl_call_wgsl.txt b/src/compose/tests/expected/glsl_call_wgsl.txt index 5cba301..f5caede 100644 --- a/src/compose/tests/expected/glsl_call_wgsl.txt +++ b/src/compose/tests/expected/glsl_call_wgsl.txt @@ -4,12 +4,12 @@ struct VertexOutput { var gl_Position: vec4; -fn _naga_oil_mod_O5TXG3C7NVXWI5LMMU_memberwgsl_func() -> f32 { +fn wgsl_funcX_naga_oil_mod_XO5TXG3C7NVXWI5LMMUX() -> f32 { return 53.0; } fn main_1() { - let _e0: f32 = _naga_oil_mod_O5TXG3C7NVXWI5LMMU_memberwgsl_func(); + let _e0: f32 = wgsl_funcX_naga_oil_mod_XO5TXG3C7NVXWI5LMMUX(); gl_Position = vec4(_e0); return; } diff --git a/src/compose/tests/expected/glsl_const_import.txt b/src/compose/tests/expected/glsl_const_import.txt index b39a96f..f46c308 100644 --- a/src/compose/tests/expected/glsl_const_import.txt +++ b/src/compose/tests/expected/glsl_const_import.txt @@ -2,16 +2,12 @@ struct FragmentOutput { @location(0) out_color: vec4, } -const _naga_oil_mod_MNXW23LPNY_membermy_constant: f32 = 0.5; +const my_constantX_naga_oil_mod_XMNXW23LPNYX: f32 = 0.5; var out_color: vec4; -fn _naga_oil_mod_MNXW23LPNY_membermain() { - return; -} - fn main_1() { - out_color = vec4(f32(1), _naga_oil_mod_MNXW23LPNY_membermy_constant, f32(0), f32(1)); + out_color = vec4(f32(1), my_constantX_naga_oil_mod_XMNXW23LPNYX, f32(0), f32(1)); return; } diff --git a/src/compose/tests/expected/glsl_wgsl_const_import.txt b/src/compose/tests/expected/glsl_wgsl_const_import.txt index ed543e4..637c511 100644 --- a/src/compose/tests/expected/glsl_wgsl_const_import.txt +++ b/src/compose/tests/expected/glsl_wgsl_const_import.txt @@ -1,10 +1,6 @@ -const _naga_oil_mod_MNXW23LPNY_membermy_constant: f32 = 0.5; - -fn _naga_oil_mod_MNXW23LPNY_membermain() { - return; -} +const my_constantX_naga_oil_mod_XMNXW23LPNYX: f32 = 0.5; fn main() -> vec4 { - return vec4(1.0, _naga_oil_mod_MNXW23LPNY_membermy_constant, 0.0, 1.0); + return vec4(1.0, my_constantX_naga_oil_mod_XMNXW23LPNYX, 0.0, 1.0); } diff --git a/src/compose/tests/expected/import_in_decl.txt b/src/compose/tests/expected/import_in_decl.txt index b7cdc68..4ecba51 100644 --- a/src/compose/tests/expected/import_in_decl.txt +++ b/src/compose/tests/expected/import_in_decl.txt @@ -1,11 +1,9 @@ -const _naga_oil_mod_MNXW443UOM_memberX: u32 = 1u; +const XX_naga_oil_mod_XMNXW443UOMX: u32 = 1u; -const _naga_oil_mod_MJUW4ZA_membery: u32 = 2u; - -var _naga_oil_mod_MJUW4ZA_memberarr: array; +var arrX_naga_oil_mod_XMJUW4ZAX: array; fn main() -> f32 { - let _e2: u32 = _naga_oil_mod_MJUW4ZA_memberarr[0]; + let _e2: u32 = arrX_naga_oil_mod_XMJUW4ZAX[0]; return f32(_e2); } diff --git a/src/compose/tests/expected/invalid_override_base.txt b/src/compose/tests/expected/invalid_override_base.txt index a0ee0a4..f2eb81a 100644 --- a/src/compose/tests/expected/invalid_override_base.txt +++ b/src/compose/tests/expected/invalid_override_base.txt @@ -1,6 +1,6 @@ error: override is invalid as `outer` is not virtual (this error can be disabled with feature 'override_any') - โ”Œโ”€ tests/overrides/top_invalid.wgsl:3:39 + โ”Œโ”€ tests/overrides/top_invalid.wgsl:3:13 โ”‚ 3 โ”‚ override fn mod::outer() -> f32 { - โ”‚ ^ + โ”‚ ^ diff --git a/src/compose/tests/expected/item_import_test.txt b/src/compose/tests/expected/item_import_test.txt index f7a23ce..60c3198 100644 --- a/src/compose/tests/expected/item_import_test.txt +++ b/src/compose/tests/expected/item_import_test.txt @@ -3,14 +3,14 @@ - let _e1: u32 = _naga_oil_mod_MNXW443UOM_memberdouble(_naga_oil_mod_MNXW443UOM_memberX); - let _e1: u32 = _naga_oil_mod_MNXW443UOM_memberdouble(_naga_oil_mod_MNXW443UOM_memberY); + let _e1: u32 = doubleX_naga_oil_mod_XMNXW443UOMX(XX_naga_oil_mod_XMNXW443UOMX); + let _e1: u32 = doubleX_naga_oil_mod_XMNXW443UOMX(YX_naga_oil_mod_XMNXW443UOMX); return (in * 2u); return _e1; return _e1; -const _naga_oil_mod_MNXW443UOM_memberX: u32 = 1u; -const _naga_oil_mod_MNXW443UOM_memberY: u32 = 2u; -fn _naga_oil_mod_MNXW443UOM_memberdouble(in: u32) -> u32 { +const XX_naga_oil_mod_XMNXW443UOMX: u32 = 1u; +const YX_naga_oil_mod_XMNXW443UOMX: u32 = 2u; +fn doubleX_naga_oil_mod_XMNXW443UOMX(in: u32) -> u32 { fn main() -> u32 { fn other() -> u32 { } diff --git a/src/compose/tests/expected/item_sub_point.txt b/src/compose/tests/expected/item_sub_point.txt index 8b89742..63daa0f 100644 --- a/src/compose/tests/expected/item_sub_point.txt +++ b/src/compose/tests/expected/item_sub_point.txt @@ -1,17 +1,17 @@ -struct _naga_oil_mod_NVXWI_memberFrag { +struct FragX_naga_oil_mod_XNVXWIX { fragment: f32, } -fn _naga_oil_mod_NVXWI_memberfragment(f_1: _naga_oil_mod_NVXWI_memberFrag) -> f32 { +fn fragmentX_naga_oil_mod_XNVXWIX(f_1: FragX_naga_oil_mod_XNVXWIX) -> f32 { return (f_1.fragment * 2.0); } @fragment fn main() -> @location(0) f32 { - var f: _naga_oil_mod_NVXWI_memberFrag; + var f: FragX_naga_oil_mod_XNVXWIX; f.fragment = 3.0; - let _e3: _naga_oil_mod_NVXWI_memberFrag = f; - let _e4: f32 = _naga_oil_mod_NVXWI_memberfragment(_e3); + let _e3: FragX_naga_oil_mod_XNVXWIX = f; + let _e4: f32 = fragmentX_naga_oil_mod_XNVXWIX(_e3); return _e4; } diff --git a/src/compose/tests/expected/missing_import.txt b/src/compose/tests/expected/missing_import.txt index cf4751f..f08bccb 100644 --- a/src/compose/tests/expected/missing_import.txt +++ b/src/compose/tests/expected/missing_import.txt @@ -1,8 +1,8 @@ error: required import 'missing' not found - โ”Œโ”€ tests/error_test/include.wgsl:7:1 - โ”‚ -7 โ”‚ #import missing - โ”‚ ^ - โ”‚ - = missing import 'missing' + โ”Œโ”€ tests/error_test/include.wgsl:11:13 + โ”‚ +11 โ”‚ let x = missing::y(); + โ”‚ ^ + โ”‚ + = missing import 'missing' diff --git a/src/compose/tests/expected/simple_compose.txt b/src/compose/tests/expected/simple_compose.txt index 3c97a9f..d8aacaa 100644 --- a/src/compose/tests/expected/simple_compose.txt +++ b/src/compose/tests/expected/simple_compose.txt @@ -1,9 +1,9 @@ -fn _naga_oil_mod_NFXGG_memberhello() -> f32 { +fn helloX_naga_oil_mod_XNFXGGX() -> f32 { return 1.0; } fn main() -> f32 { - let _e0: f32 = _naga_oil_mod_NFXGG_memberhello(); + let _e0: f32 = helloX_naga_oil_mod_XNFXGGX(); return _e0; } diff --git a/src/compose/tests/expected/wgsl_call_entrypoint.txt b/src/compose/tests/expected/wgsl_call_entrypoint.txt index 801fec1..8df9240 100644 --- a/src/compose/tests/expected/wgsl_call_entrypoint.txt +++ b/src/compose/tests/expected/wgsl_call_entrypoint.txt @@ -1,13 +1,9 @@ -fn _naga_oil_mod_NFXGG3DVMRSQ_membernon_ep(f: f32) -> f32 { - return (f * 2.0); -} - -fn _naga_oil_mod_NFXGG3DVMRSQ_memberfragment(frag_coord_1: vec4) -> vec4 { - return vec4(1.0); +fn fragmentX_naga_oil_mod_XNFXGG3DVMRSQX(frag_coord_1: vec4) -> vec4 { + return vec4((1.5 * frag_coord_1)); } @fragment fn fragment(@builtin(position) frag_coord: vec4) -> @location(0) vec4 { - let _e1: vec4 = _naga_oil_mod_NFXGG3DVMRSQ_memberfragment(frag_coord); + let _e1: vec4 = fragmentX_naga_oil_mod_XNFXGG3DVMRSQX(frag_coord); return _e1; } diff --git a/src/compose/tests/expected/wgsl_glsl_const_import.txt b/src/compose/tests/expected/wgsl_glsl_const_import.txt index d9fcd57..f46c308 100644 --- a/src/compose/tests/expected/wgsl_glsl_const_import.txt +++ b/src/compose/tests/expected/wgsl_glsl_const_import.txt @@ -2,12 +2,12 @@ struct FragmentOutput { @location(0) out_color: vec4, } -const _naga_oil_mod_MNXW23LPNY_membermy_constant: f32 = 0.5; +const my_constantX_naga_oil_mod_XMNXW23LPNYX: f32 = 0.5; var out_color: vec4; fn main_1() { - out_color = vec4(f32(1), _naga_oil_mod_MNXW23LPNY_membermy_constant, f32(0), f32(1)); + out_color = vec4(f32(1), my_constantX_naga_oil_mod_XMNXW23LPNYX, f32(0), f32(1)); return; } diff --git a/src/compose/tests/invalid_identifiers/top_invalid.wgsl b/src/compose/tests/invalid_identifiers/top_invalid.wgsl new file mode 100644 index 0000000..93c68f2 --- /dev/null +++ b/src/compose/tests/invalid_identifiers/top_invalid.wgsl @@ -0,0 +1,23 @@ +// #import consts +// #import fns +// #import globals +#import struct_members +// #import structs + +fn main() -> f32 { + // let a = consts::fine + consts::bad_; + // let b = fns::fine(1.0) + fns::bad_(2.0); + // let c = globals::fine + globals::bad_; + // var d: structs::IsFine; + // d.fine = 3.0; + // var e: structs::Isbad_; + // e.fine_member = 4.0; + var f = struct_members::FineStruct; + f.fine = 5.0; + var g = struct_members::BadStruct; + g.also_fine = 6.0; + g.bad_ = 7.0; + + // return a + b + c + d.fine + e.fine_member; + return f.fine + g.also_fine + g.bad_; +} \ No newline at end of file diff --git a/src/compose/tests/invalid_identifiers/top_valid.wgsl b/src/compose/tests/invalid_identifiers/top_valid.wgsl new file mode 100644 index 0000000..f0753b7 --- /dev/null +++ b/src/compose/tests/invalid_identifiers/top_valid.wgsl @@ -0,0 +1,22 @@ +#import consts +#import fns +#import globals +// #import struct_members +#import structs + +fn main() -> f32 { + let a = consts::fine + consts::bad_; + let b = fns::fine(1.0) + fns::bad_(2.0); + let c = globals::fine + globals::bad_; + var d: structs::IsFine; + d.fine = 3.0; + var e: structs::Isbad_; + e.fine_member = 4.0; + // var f = struct_members::FineStruct; + // f.fine = 5.0; + // var g = struct_members::BadStruct; + // g.also_fine = 6.0; + // g.bad_ = 7.0; + + return a + b + c + d.fine + e.fine_member; // + f.fine + g.also_fine + g.bad_; +} \ No newline at end of file diff --git a/src/compose/tests/rusty_imports/mod_a_b_c.wgsl b/src/compose/tests/rusty_imports/mod_a_b_c.wgsl new file mode 100644 index 0000000..e8e0abc --- /dev/null +++ b/src/compose/tests/rusty_imports/mod_a_b_c.wgsl @@ -0,0 +1,7 @@ +#define_import_path a::b::c + +const C: f32 = 2.0; + +fn triple(in: f32) -> f32 { + return in * 3.0; +} \ No newline at end of file diff --git a/src/compose/tests/rusty_imports/mod_a_x.wgsl b/src/compose/tests/rusty_imports/mod_a_x.wgsl new file mode 100644 index 0000000..91a917e --- /dev/null +++ b/src/compose/tests/rusty_imports/mod_a_x.wgsl @@ -0,0 +1,5 @@ +#define_import_path a::x + +fn square(in: f32) -> f32 { + return in * in; +} \ No newline at end of file diff --git a/src/compose/tests/rusty_imports/top.wgsl b/src/compose/tests/rusty_imports/top.wgsl new file mode 100644 index 0000000..2ee148f --- /dev/null +++ b/src/compose/tests/rusty_imports/top.wgsl @@ -0,0 +1,8 @@ +#define_import_path test_module + +#import a::b as partial_path +#import a::b::c as full_path + +fn entry_point() -> f32 { + return a::x::square(partial_path::c::triple(full_path::C)); +} \ No newline at end of file From a95c44401a4adcaf7d86b8a92b53dc53755b919a Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 22:49:59 +0100 Subject: [PATCH 03/30] remove debug println --- src/compose/ident_identifier.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compose/ident_identifier.rs b/src/compose/ident_identifier.rs index cb80c86..142fa33 100644 --- a/src/compose/ident_identifier.rs +++ b/src/compose/ident_identifier.rs @@ -213,7 +213,6 @@ pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &Has if let Some((module, item)) = full_path.rsplit_once("::") { used_imports.entry(module.to_owned()).or_insert_with(|| { - println!("setting import offset for {module} to {offset}+{token_pos} = {}", offset + token_pos); ImportDefWithOffset { definition: ImportDefinition { import: module.to_owned(), ..Default::default() }, offset: offset + token_pos } }).definition.items.push(item.to_owned()); output.push_str(item); From f88db5d70ff2dd85db0dc5423283dd543f085dd0 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 22:53:28 +0100 Subject: [PATCH 04/30] reorg --- src/compose/mod.rs | 3 +- .../{ident_identifier.rs => parse_imports.rs} | 104 +----------------- src/compose/preprocess.rs | 2 +- src/compose/tokenizer.rs | 102 +++++++++++++++++ 4 files changed, 107 insertions(+), 104 deletions(-) rename src/compose/{ident_identifier.rs => parse_imports.rs} (70%) create mode 100644 src/compose/tokenizer.rs diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 3a034e9..a90d0c0 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -143,7 +143,8 @@ pub mod comment_strip_iter; pub mod error; pub mod preprocess; mod test; -pub mod ident_identifier; +pub mod parse_imports; +pub mod tokenizer; #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug, Default)] pub enum ShaderLanguage { diff --git a/src/compose/ident_identifier.rs b/src/compose/parse_imports.rs similarity index 70% rename from src/compose/ident_identifier.rs rename to src/compose/parse_imports.rs index 142fa33..88dd28c 100644 --- a/src/compose/ident_identifier.rs +++ b/src/compose/parse_imports.rs @@ -1,107 +1,7 @@ -use std::collections::{VecDeque, HashMap}; +use std::collections::HashMap; -use super::{ImportDefWithOffset, ImportDefinition, Composer}; +use super::{tokenizer::{Tokenizer, Token}, ImportDefWithOffset, ImportDefinition, Composer}; -#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] -pub enum Token<'a> { - Identifier(&'a str, usize), - Other(char, usize), - Whitespace(&'a str, usize), -} - -impl<'a> Token<'a> { - fn pos(&self) -> usize { - match self { - Token::Identifier(_, pos) | - Token::Other(_, pos) | - Token::Whitespace(_, pos) => *pos - } - } - - fn identifier(&self) -> Option<&str> { - match self { - Token::Identifier(ident, _) => Some(ident), - _ => None, - } - } -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum TokenKind { - Identifier, - Whitespace, -} - -// a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens -// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters -pub struct Tokenizer<'a> { - tokens: VecDeque>, -} - -impl<'a> Tokenizer<'a> { - pub fn new(src: &'a str, emit_whitespace: bool) -> Self{ - let mut tokens = VecDeque::default(); - let mut current_token_start = 0; - let mut current_token = None; - - // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue - for (ix, char) in src.char_indices() { - if let Some(tok) = current_token { - match tok { - TokenKind::Identifier => { - if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { - continue; - } - tokens.push_back(Token::Identifier(&src[current_token_start..ix], current_token_start)); - } - TokenKind::Whitespace => { - if char.is_whitespace() { - continue; - } - tokens.push_back(Token::Whitespace(&src[current_token_start..ix], current_token_start)); - } - }; - - current_token_start = ix; - current_token = None; - } - - if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { - current_token = Some(TokenKind::Identifier); - current_token_start = ix; - } else if !char.is_whitespace() { - tokens.push_back(Token::Other(char, current_token_start)); - } else if char.is_whitespace() && emit_whitespace { - current_token = Some(TokenKind::Whitespace); - current_token_start = ix; - } - } - - if let Some(tok) = current_token { - match tok { - TokenKind::Identifier => { - tokens.push_back(Token::Identifier(&src[current_token_start..src.len()], current_token_start)); - } - TokenKind::Whitespace => { - tokens.push_back(Token::Whitespace(&src[current_token_start..src.len()], current_token_start)); - } - }; - } - - Self { - tokens, - } - } -} - -impl<'a> Iterator for Tokenizer<'a> { - type Item = Token<'a>; - - fn next(&mut self) -> Option { - let tok = self.tokens.pop_front(); - tok - } -} pub fn parse_imports<'a>(input: &'a str, declared_imports: &mut HashMap>) -> Result<(), (&'a str, usize)> { let mut tokens = Tokenizer::new(input, false).peekable(); diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 8bb9223..f46c743 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -4,7 +4,7 @@ use regex::Regex; use super::{ comment_strip_iter::CommentReplaceExt, ComposerErrorInner, ImportDefWithOffset, - ShaderDefValue, ident_identifier::{parse_imports, substitute_identifiers}, + ShaderDefValue, parse_imports::{parse_imports, substitute_identifiers}, }; #[derive(Debug)] diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs new file mode 100644 index 0000000..f6984bc --- /dev/null +++ b/src/compose/tokenizer.rs @@ -0,0 +1,102 @@ +use std::collections::VecDeque; + +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub enum Token<'a> { + Identifier(&'a str, usize), + Other(char, usize), + Whitespace(&'a str, usize), +} + +impl<'a> Token<'a> { + pub fn pos(&self) -> usize { + match self { + Token::Identifier(_, pos) | + Token::Other(_, pos) | + Token::Whitespace(_, pos) => *pos + } + } + + pub fn identifier(&self) -> Option<&str> { + match self { + Token::Identifier(ident, _) => Some(ident), + _ => None, + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum TokenKind { + Identifier, + Whitespace, +} + +// a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens +// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters +pub struct Tokenizer<'a> { + tokens: VecDeque>, +} + +impl<'a> Tokenizer<'a> { + pub fn new(src: &'a str, emit_whitespace: bool) -> Self{ + let mut tokens = VecDeque::default(); + let mut current_token_start = 0; + let mut current_token = None; + + // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue + for (ix, char) in src.char_indices() { + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { + continue; + } + tokens.push_back(Token::Identifier(&src[current_token_start..ix], current_token_start)); + } + TokenKind::Whitespace => { + if char.is_whitespace() { + continue; + } + tokens.push_back(Token::Whitespace(&src[current_token_start..ix], current_token_start)); + } + }; + + current_token_start = ix; + current_token = None; + } + + if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { + current_token = Some(TokenKind::Identifier); + current_token_start = ix; + } else if !char.is_whitespace() { + tokens.push_back(Token::Other(char, current_token_start)); + } else if char.is_whitespace() && emit_whitespace { + current_token = Some(TokenKind::Whitespace); + current_token_start = ix; + } + } + + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + tokens.push_back(Token::Identifier(&src[current_token_start..src.len()], current_token_start)); + } + TokenKind::Whitespace => { + tokens.push_back(Token::Whitespace(&src[current_token_start..src.len()], current_token_start)); + } + }; + } + + Self { + tokens, + } + } +} + +impl<'a> Iterator for Tokenizer<'a> { + type Item = Token<'a>; + + fn next(&mut self) -> Option { + let tok = self.tokens.pop_front(); + tok + } +} From 3cb1a22434fa099bfe5f94394e1ae4db63a06fee Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 22:53:46 +0100 Subject: [PATCH 05/30] fmt --- src/compose/error.rs | 5 +- src/compose/mod.rs | 63 ++-- src/compose/parse_imports.rs | 590 +++++++++++++++++++---------------- src/compose/preprocess.rs | 98 ++++-- src/compose/test.rs | 29 +- src/compose/tokenizer.rs | 212 +++++++------ 6 files changed, 565 insertions(+), 432 deletions(-) diff --git a/src/compose/error.rs b/src/compose/error.rs index 48d446d..604e376 100644 --- a/src/compose/error.rs +++ b/src/compose/error.rs @@ -8,10 +8,7 @@ use codespan_reporting::{ use thiserror::Error; use tracing::trace; -use super::{ - preprocess::PreprocessOutput, - Composer, ShaderDefValue, -}; +use super::{preprocess::PreprocessOutput, Composer, ShaderDefValue}; use crate::{compose::SPAN_SHIFT, redirect::RedirectError}; #[derive(Debug)] diff --git a/src/compose/mod.rs b/src/compose/mod.rs index a90d0c0..e337a2e 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -141,9 +141,9 @@ use self::preprocess::Preprocessor; pub mod comment_strip_iter; pub mod error; +pub mod parse_imports; pub mod preprocess; mod test; -pub mod parse_imports; pub mod tokenizer; #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug, Default)] @@ -337,7 +337,15 @@ impl Default for Composer { module_sets: Default::default(), module_index: Default::default(), preprocessor: Preprocessor::default(), - check_decoration_regex: Regex::new(format!("({}|{})", regex_syntax::escape(DECORATION_PRE), regex_syntax::escape(DECORATION_OVERRIDE_PRE)).as_str()).unwrap(), + check_decoration_regex: Regex::new( + format!( + "({}|{})", + regex_syntax::escape(DECORATION_PRE), + regex_syntax::escape(DECORATION_OVERRIDE_PRE) + ) + .as_str(), + ) + .unwrap(), undecorate_regex: Regex::new( format!( r"([\w\d_]+){}([A-Z0-9]*){}", @@ -347,15 +355,19 @@ impl Default for Composer { .as_str(), ) .unwrap(), - virtual_fn_regex: Regex::new(r"(?P[\s]*virtual\s+fn\s+)(?P[^\s]+)(?P\s*)\(").unwrap(), + virtual_fn_regex: Regex::new( + r"(?P[\s]*virtual\s+fn\s+)(?P[^\s]+)(?P\s*)\(", + ) + .unwrap(), override_fn_regex: Regex::new( format!( - r"(override\s+fn\s+)([^\s]+){}([\w\d]+){}(\s*)\(", + r"(override\s+fn\s+)([^\s]+){}([\w\d]+){}(\s*)\(", regex_syntax::escape(DECORATION_PRE), regex_syntax::escape(DECORATION_POST) ) - .as_str() - ).unwrap(), + .as_str(), + ) + .unwrap(), undecorate_override_regex: Regex::new( format!( "{}([A-Z0-9]*){}", @@ -395,7 +407,7 @@ impl Composer { let encoded = data_encoding::BASE32_NOPAD.encode(module.as_bytes()); format!("{DECORATION_PRE}{encoded}{DECORATION_POST}") } - + fn decode(from: &str) -> String { String::from_utf8(data_encoding::BASE32_NOPAD.decode(from.as_bytes()).unwrap()).unwrap() } @@ -404,7 +416,11 @@ impl Composer { let undecor = self .undecorate_regex .replace_all(string, |caps: ®ex::Captures| { - format!("{}::{}", Self::decode(caps.get(2).unwrap().as_str()), caps.get(1).unwrap().as_str()) + format!( + "{}::{}", + Self::decode(caps.get(2).unwrap().as_str()), + caps.get(1).unwrap().as_str() + ) }); let undecor = @@ -745,8 +761,7 @@ impl Composer { .collect(); for (h, c) in source_ir.constants.iter() { if let Some(name) = &c.name { - if name.ends_with(module_decoration) && !recompiled_consts.contains(name.as_str()) - { + if name.ends_with(module_decoration) && !recompiled_consts.contains(name.as_str()) { return Err(ComposerErrorInner::InvalidIdentifier { original: name.clone(), at: source_ir.constants.get_span(h), @@ -763,8 +778,7 @@ impl Composer { .collect(); for (h, gv) in source_ir.global_variables.iter() { if let Some(name) = &gv.name { - if name.ends_with(module_decoration) - && !recompiled_globals.contains(name.as_str()) + if name.ends_with(module_decoration) && !recompiled_globals.contains(name.as_str()) { return Err(ComposerErrorInner::InvalidIdentifier { original: name.clone(), @@ -819,7 +833,7 @@ impl Composer { let PreprocessOutput { preprocessed_source: source, - imports + imports, } = self .preprocessor .preprocess( @@ -1213,10 +1227,11 @@ impl Composer { for (h_f, f) in source_ir.functions.iter() { if let Some(name) = &f.name { if composable.owned_functions.contains(name) - && ( - items.map_or(true, |items| items.contains(name)) || - composable.override_functions.values().any(|v| v.contains(name)) - ) + && (items.map_or(true, |items| items.contains(name)) + || composable + .override_functions + .values() + .any(|v| v.contains(name))) { let span = composable.module_ir.functions.get_span(h_f); derived.import_function_if_new(f, span); @@ -1531,7 +1546,12 @@ impl Composer { let sanitized_source = self.sanitize_and_set_auto_bindings(source); - let PreprocessorMetaData { name, defines, imports, .. } = self + let PreprocessorMetaData { + name, + defines, + imports, + .. + } = self .preprocessor .get_preprocessor_metadata(&sanitized_source, true) .map_err(|inner| ComposerError { @@ -1751,7 +1771,12 @@ pub fn get_preprocessor_data( Vec, HashMap, ) { - let PreprocessorMetaData { name, imports, defines, .. } = PREPROCESSOR + let PreprocessorMetaData { + name, + imports, + defines, + .. + } = PREPROCESSOR .get_preprocessor_metadata(source, true) .unwrap(); ( diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 88dd28c..11639fe 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -1,263 +1,327 @@ -use std::collections::HashMap; - -use super::{tokenizer::{Tokenizer, Token}, ImportDefWithOffset, ImportDefinition, Composer}; - - -pub fn parse_imports<'a>(input: &'a str, declared_imports: &mut HashMap>) -> Result<(), (&'a str, usize)> { - let mut tokens = Tokenizer::new(input, false).peekable(); - - match tokens.next() { - Some(Token::Other('#', _)) => (), - Some(other) => return Err(("expected `#import`", other.pos())), - None => return Err(("expected #import", input.len())), - }; - match tokens.next() { - Some(Token::Identifier("import", _)) => (), - Some(other) => return Err(("expected `#import`", other.pos())), - None => return Err(("expected `#import`", input.len())), - }; - - let mut stack = Vec::default(); - let mut current = String::default(); - let mut as_name = None; - let mut is_deprecated_itemlist = false; - - loop { - match tokens.peek() { - Some(Token::Identifier(ident, _)) => { - current.push_str(ident); - tokens.next(); - - if tokens.peek().and_then(Token::identifier) == Some("as") { - let pos = tokens.next().unwrap().pos(); - let Some(Token::Identifier(name, _)) = tokens.next() else { - return Err(("expected identifier after `as`", pos)); - }; - - as_name = Some(name); - } - - // support deprecated #import mod item - if let Some(Token::Identifier(..)) = tokens.peek() { - is_deprecated_itemlist = true; - stack.push(format!("{}::", current)); - current = String::default(); - as_name = None; - } - - continue; - } - Some(Token::Other('{', pos)) => { - if !current.ends_with("::") { - return Err(("open brace must follow `::`", *pos)); - } - stack.push(current); - current = String::default(); - as_name = None; - } - Some(Token::Other(',', _)) | - Some(Token::Other('}', _)) | - None => { - if !current.is_empty() { - let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| current.rsplit_once("::").map(|(_, name)| name.to_owned()).unwrap_or(current.clone())); - declared_imports.entry(used_name).or_default().push(format!("{}{}", stack.join(""), current)); - current = String::default(); - as_name = None; - } - - if let Some(Token::Other('}', pos)) = tokens.peek() { - if stack.pop().is_none() { - return Err(("close brace without open", *pos)); - } - } - - if tokens.peek().is_none() { - break; - } - } - Some(Token::Other(_, pos)) => return Err(("unexpected token", *pos)), - Some(Token::Whitespace(..)) => unreachable!(), - } - - tokens.next(); - } - - if !stack.is_empty() && !(is_deprecated_itemlist && stack.len() == 1) { - return Err(("missing close brace", input.len())); - } - - Ok(()) -} - -pub fn substitute_identifiers(input: &str, offset: usize, declared_imports: &HashMap>, used_imports: &mut HashMap, allow_ambiguous: bool) -> Result { - let tokens = Tokenizer::new(input, true); - let mut output = String::with_capacity(input.len()); - let mut in_substitution_position = true; - - for token in tokens { - match token { - Token::Identifier(ident, token_pos) => { - if in_substitution_position { - let (first, residual) = ident.split_once("::").unwrap_or((ident, "")); - let full_paths = declared_imports.get(first).cloned().unwrap_or(vec![first.to_owned()]); - - if !allow_ambiguous && full_paths.len() > 1 { - return Err(offset + token_pos); - } - - for mut full_path in full_paths { - if !residual.is_empty() { - full_path.push_str("::"); - full_path.push_str(residual); - } - - if let Some((module, item)) = full_path.rsplit_once("::") { - used_imports.entry(module.to_owned()).or_insert_with(|| { - ImportDefWithOffset { definition: ImportDefinition { import: module.to_owned(), ..Default::default() }, offset: offset + token_pos } - }).definition.items.push(item.to_owned()); - output.push_str(item); - output.push_str(&Composer::decorate(module)); - } else { - output.push_str(&full_path); - } - } - } else { - output.push_str(ident); - } - }, - Token::Other(other, _) => { - output.push(other); - if other == '.' || other == '@' { - in_substitution_position = false; - continue; - } - } - Token::Whitespace(ws, _) => output.push_str(ws), - } - - in_substitution_position = true; - } - - Ok(output) -} - -#[cfg(test)] -fn test_parse(input: &str) -> Result>, (&str, usize)> { - let mut declared_imports = HashMap::default(); - parse_imports(input, &mut declared_imports)?; - Ok(declared_imports) -} - -#[test] -fn import_tokens() { - let input = r" - #import a::b - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([("b".to_owned(), vec!("a::b".to_owned()))]))); - - let input = r" - #import a::{b, c} - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("b".to_owned(), vec!("a::b".to_owned())), - ("c".to_owned(), vec!("a::c".to_owned())), - ]))); - - let input = r" - #import a::{b as d, c} - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("d".to_owned(), vec!("a::b".to_owned())), - ("c".to_owned(), vec!("a::c".to_owned())), - ]))); - - let input = r" - #import a::{b::{c, d}, e} - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("c".to_owned(), vec!("a::b::c".to_owned())), - ("d".to_owned(), vec!("a::b::d".to_owned())), - ("e".to_owned(), vec!("a::e".to_owned())), - ]))); - - let input = r" - #import a::b::{c, d}, e - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("c".to_owned(), vec!("a::b::c".to_owned())), - ("d".to_owned(), vec!("a::b::d".to_owned())), - ("e".to_owned(), vec!("e".to_owned())), - ]))); - - let input = r" - #import a, b - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("a".to_owned(), vec!("a".to_owned())), - ("b".to_owned(), vec!("b".to_owned())), - ]))); - - let input = r" - #import a::b c, d - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("c".to_owned(), vec!("a::b::c".to_owned())), - ("d".to_owned(), vec!("a::b::d".to_owned())), - ]))); - - let input = r" - #import a::b c - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("c".to_owned(), vec!("a::b::c".to_owned())), - ]))); - - let input = r" - #import a::b::{c::{d, e}, f, g::{h as i, j}} - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("d".to_owned(), vec!("a::b::c::d".to_owned())), - ("e".to_owned(), vec!("a::b::c::e".to_owned())), - ("f".to_owned(), vec!("a::b::f".to_owned())), - ("i".to_owned(), vec!("a::b::g::h".to_owned())), - ("j".to_owned(), vec!("a::b::g::j".to_owned())), - ]))); - - let input = r" - #import a::b::{ - c::{d, e}, - f, - g::{ - h as i, - j::k::l as m, - } - } - "; - assert_eq!(test_parse(input), Ok(HashMap::from_iter([ - ("d".to_owned(), vec!("a::b::c::d".to_owned())), - ("e".to_owned(), vec!("a::b::c::e".to_owned())), - ("f".to_owned(), vec!("a::b::f".to_owned())), - ("i".to_owned(), vec!("a::b::g::h".to_owned())), - ("m".to_owned(), vec!("a::b::g::j::k::l".to_owned())), - ]))); - - let input = r" - #import a::b::{ - "; - assert!(test_parse(input).is_err()); - - let input = r" - #import a::b::{c}} - "; - assert!(test_parse(input).is_err()); - - let input = r" - #import a::b::{c}} - "; - assert!(test_parse(input).is_err()); - - let input = r" - #import a::b{{c,d}} - "; - assert!(test_parse(input).is_err()); -} +use std::collections::HashMap; + +use super::{ + tokenizer::{Token, Tokenizer}, + Composer, ImportDefWithOffset, ImportDefinition, +}; + +pub fn parse_imports<'a>( + input: &'a str, + declared_imports: &mut HashMap>, +) -> Result<(), (&'a str, usize)> { + let mut tokens = Tokenizer::new(input, false).peekable(); + + match tokens.next() { + Some(Token::Other('#', _)) => (), + Some(other) => return Err(("expected `#import`", other.pos())), + None => return Err(("expected #import", input.len())), + }; + match tokens.next() { + Some(Token::Identifier("import", _)) => (), + Some(other) => return Err(("expected `#import`", other.pos())), + None => return Err(("expected `#import`", input.len())), + }; + + let mut stack = Vec::default(); + let mut current = String::default(); + let mut as_name = None; + let mut is_deprecated_itemlist = false; + + loop { + match tokens.peek() { + Some(Token::Identifier(ident, _)) => { + current.push_str(ident); + tokens.next(); + + if tokens.peek().and_then(Token::identifier) == Some("as") { + let pos = tokens.next().unwrap().pos(); + let Some(Token::Identifier(name, _)) = tokens.next() else { + return Err(("expected identifier after `as`", pos)); + }; + + as_name = Some(name); + } + + // support deprecated #import mod item + if let Some(Token::Identifier(..)) = tokens.peek() { + is_deprecated_itemlist = true; + stack.push(format!("{}::", current)); + current = String::default(); + as_name = None; + } + + continue; + } + Some(Token::Other('{', pos)) => { + if !current.ends_with("::") { + return Err(("open brace must follow `::`", *pos)); + } + stack.push(current); + current = String::default(); + as_name = None; + } + Some(Token::Other(',', _)) | Some(Token::Other('}', _)) | None => { + if !current.is_empty() { + let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| { + current + .rsplit_once("::") + .map(|(_, name)| name.to_owned()) + .unwrap_or(current.clone()) + }); + declared_imports.entry(used_name).or_default().push(format!( + "{}{}", + stack.join(""), + current + )); + current = String::default(); + as_name = None; + } + + if let Some(Token::Other('}', pos)) = tokens.peek() { + if stack.pop().is_none() { + return Err(("close brace without open", *pos)); + } + } + + if tokens.peek().is_none() { + break; + } + } + Some(Token::Other(_, pos)) => return Err(("unexpected token", *pos)), + Some(Token::Whitespace(..)) => unreachable!(), + } + + tokens.next(); + } + + if !stack.is_empty() && !(is_deprecated_itemlist && stack.len() == 1) { + return Err(("missing close brace", input.len())); + } + + Ok(()) +} + +pub fn substitute_identifiers( + input: &str, + offset: usize, + declared_imports: &HashMap>, + used_imports: &mut HashMap, + allow_ambiguous: bool, +) -> Result { + let tokens = Tokenizer::new(input, true); + let mut output = String::with_capacity(input.len()); + let mut in_substitution_position = true; + + for token in tokens { + match token { + Token::Identifier(ident, token_pos) => { + if in_substitution_position { + let (first, residual) = ident.split_once("::").unwrap_or((ident, "")); + let full_paths = declared_imports + .get(first) + .cloned() + .unwrap_or(vec![first.to_owned()]); + + if !allow_ambiguous && full_paths.len() > 1 { + return Err(offset + token_pos); + } + + for mut full_path in full_paths { + if !residual.is_empty() { + full_path.push_str("::"); + full_path.push_str(residual); + } + + if let Some((module, item)) = full_path.rsplit_once("::") { + used_imports + .entry(module.to_owned()) + .or_insert_with(|| ImportDefWithOffset { + definition: ImportDefinition { + import: module.to_owned(), + ..Default::default() + }, + offset: offset + token_pos, + }) + .definition + .items + .push(item.to_owned()); + output.push_str(item); + output.push_str(&Composer::decorate(module)); + } else { + output.push_str(&full_path); + } + } + } else { + output.push_str(ident); + } + } + Token::Other(other, _) => { + output.push(other); + if other == '.' || other == '@' { + in_substitution_position = false; + continue; + } + } + Token::Whitespace(ws, _) => output.push_str(ws), + } + + in_substitution_position = true; + } + + Ok(output) +} + +#[cfg(test)] +fn test_parse(input: &str) -> Result>, (&str, usize)> { + let mut declared_imports = HashMap::default(); + parse_imports(input, &mut declared_imports)?; + Ok(declared_imports) +} + +#[test] +fn import_tokens() { + let input = r" + #import a::b + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([( + "b".to_owned(), + vec!("a::b".to_owned()) + )])) + ); + + let input = r" + #import a::{b, c} + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("b".to_owned(), vec!("a::b".to_owned())), + ("c".to_owned(), vec!("a::c".to_owned())), + ])) + ); + + let input = r" + #import a::{b as d, c} + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b".to_owned())), + ("c".to_owned(), vec!("a::c".to_owned())), + ])) + ); + + let input = r" + #import a::{b::{c, d}, e} + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), + ("e".to_owned(), vec!("a::e".to_owned())), + ])) + ); + + let input = r" + #import a::b::{c, d}, e + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), + ("e".to_owned(), vec!("e".to_owned())), + ])) + ); + + let input = r" + #import a, b + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("a".to_owned(), vec!("a".to_owned())), + ("b".to_owned(), vec!("b".to_owned())), + ])) + ); + + let input = r" + #import a::b c, d + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("c".to_owned(), vec!("a::b::c".to_owned())), + ("d".to_owned(), vec!("a::b::d".to_owned())), + ])) + ); + + let input = r" + #import a::b c + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([( + "c".to_owned(), + vec!("a::b::c".to_owned()) + ),])) + ); + + let input = r" + #import a::b::{c::{d, e}, f, g::{h as i, j}} + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b::c::d".to_owned())), + ("e".to_owned(), vec!("a::b::c::e".to_owned())), + ("f".to_owned(), vec!("a::b::f".to_owned())), + ("i".to_owned(), vec!("a::b::g::h".to_owned())), + ("j".to_owned(), vec!("a::b::g::j".to_owned())), + ])) + ); + + let input = r" + #import a::b::{ + c::{d, e}, + f, + g::{ + h as i, + j::k::l as m, + } + } + "; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ("d".to_owned(), vec!("a::b::c::d".to_owned())), + ("e".to_owned(), vec!("a::b::c::e".to_owned())), + ("f".to_owned(), vec!("a::b::f".to_owned())), + ("i".to_owned(), vec!("a::b::g::h".to_owned())), + ("m".to_owned(), vec!("a::b::g::j::k::l".to_owned())), + ])) + ); + + let input = r" + #import a::b::{ + "; + assert!(test_parse(input).is_err()); + + let input = r" + #import a::b::{c}} + "; + assert!(test_parse(input).is_err()); + + let input = r" + #import a::b::{c}} + "; + assert!(test_parse(input).is_err()); + + let input = r" + #import a::b{{c,d}} + "; + assert!(test_parse(input).is_err()); +} diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index f46c743..bba850e 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -3,8 +3,9 @@ use std::collections::{HashMap, HashSet}; use regex::Regex; use super::{ - comment_strip_iter::CommentReplaceExt, ComposerErrorInner, ImportDefWithOffset, - ShaderDefValue, parse_imports::{parse_imports, substitute_identifiers}, + comment_strip_iter::CommentReplaceExt, + parse_imports::{parse_imports, substitute_identifiers}, + ComposerErrorInner, ImportDefWithOffset, ShaderDefValue, }; #[derive(Debug)] @@ -132,7 +133,7 @@ pub struct PreprocessOutput { impl Preprocessor { fn check_scope<'a>( - &self, + &self, shader_defs: &HashMap, line: &'a str, scope: Option<&mut Scope>, @@ -181,21 +182,18 @@ impl Preprocessor { } } - let def_value = - shader_defs - .get(def) - .ok_or(ComposerErrorInner::UnknownShaderDef { - pos: offset, - shader_def_name: def.to_string(), - })?; - - let invalid_def = |ty: &str| { - ComposerErrorInner::InvalidShaderDefComparisonValue { + let def_value = shader_defs + .get(def) + .ok_or(ComposerErrorInner::UnknownShaderDef { pos: offset, shader_def_name: def.to_string(), - value: val.as_str().to_string(), - expected: ty.to_string(), - } + })?; + + let invalid_def = |ty: &str| ComposerErrorInner::InvalidShaderDefComparisonValue { + pos: offset, + shader_def_name: def.to_string(), + value: val.as_str().to_string(), + expected: ty.to_string(), }; let new_scope = match def_value { @@ -247,10 +245,7 @@ impl Preprocessor { // this code broadly stolen from bevy_render::ShaderProcessor let mut lines = shader_str.lines(); - let mut lines = lines - .replace_comments() - .zip(shader_str.lines()) - .peekable(); + let mut lines = lines.replace_comments().zip(shader_str.lines()).peekable(); while let Some((mut line, original_line)) = lines.next() { let mut output = false; @@ -260,7 +255,10 @@ impl Preprocessor { if v != "440" && v != "450" { return Err(ComposerErrorInner::GlslInvalidVersion(offset)); } - } else if self.check_scope(shader_defs, &line, Some(&mut scope), offset)?.0 { + } else if self + .check_scope(shader_defs, &line, Some(&mut scope), offset)? + .0 + { // ignore } else if self.define_import_path_regex.captures(&line).is_some() { // ignore @@ -288,7 +286,11 @@ impl Preprocessor { line = lines.next().unwrap().0; } - parse_imports(import_lines.as_str(), &mut declared_imports).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))?; + parse_imports(import_lines.as_str(), &mut declared_imports).map_err( + |(err, offset)| { + ComposerErrorInner::ImportParseError(err.to_owned(), offset) + }, + )?; output = true; } else { let replaced_lines = [original_line, &line].map(|input| { @@ -318,10 +320,28 @@ impl Preprocessor { let decommented_line = &replaced_lines[1]; // we don't want to capture imports from comments so we run using a dummy used_imports, and disregard any errors - let item_replaced_line = substitute_identifiers(original_line, offset, &declared_imports, &mut Default::default(), true).unwrap(); + let item_replaced_line = substitute_identifiers( + original_line, + offset, + &declared_imports, + &mut Default::default(), + true, + ) + .unwrap(); // we also run against the de-commented line to replace real imports, and throw an error if appropriate - let _ = substitute_identifiers(decommented_line, offset, &declared_imports, &mut used_imports, false) - .map_err(|pos| ComposerErrorInner::ImportParseError("Ambiguous import path for item".to_owned(), pos))?; + let _ = substitute_identifiers( + decommented_line, + offset, + &declared_imports, + &mut used_imports, + false, + ) + .map_err(|pos| { + ComposerErrorInner::ImportParseError( + "Ambiguous import path for item".to_owned(), + pos, + ) + })?; final_string.push_str(&item_replaced_line); let diff = line.len().saturating_sub(item_replaced_line.len()); @@ -351,7 +371,7 @@ impl Preprocessor { Ok(PreprocessOutput { preprocessed_source: final_string, - imports: used_imports.into_values().collect() + imports: used_imports.into_values().collect(), }) } @@ -373,7 +393,7 @@ impl Preprocessor { while let Some(mut line) = lines.next() { let (is_scope, def) = self.check_scope(&HashMap::default(), &line, None, offset)?; - + if is_scope { if let Some(def) = def { effective_defs.insert(def.to_owned()); @@ -398,7 +418,9 @@ impl Preprocessor { line = lines.next().unwrap(); } - parse_imports(import_lines.as_str(), &mut declared_imports).map_err(|(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset))?; + parse_imports(import_lines.as_str(), &mut declared_imports).map_err( + |(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset), + )?; } else if let Some(cap) = self.define_import_path_regex.captures(&line) { name = Some(cap.get(1).unwrap().as_str().to_string()); } else if let Some(cap) = self.define_shader_def_regex.captures(&line) { @@ -425,13 +447,19 @@ impl Preprocessor { return Err(ComposerErrorInner::DefineInModule(offset)); } } else { - substitute_identifiers(&line, offset, &declared_imports, &mut used_imports, true).unwrap(); + substitute_identifiers(&line, offset, &declared_imports, &mut used_imports, true) + .unwrap(); } offset += line.len() + 1; } - Ok(PreprocessorMetaData { name, imports: used_imports.into_values().collect(), defines, effective_defs }) + Ok(PreprocessorMetaData { + name, + imports: used_imports.into_values().collect(), + defines, + effective_defs, + }) } } @@ -997,7 +1025,10 @@ defined "; let processor = Preprocessor::default(); - let PreprocessorMetaData { defines: shader_defs, .. } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); + let PreprocessorMetaData { + defines: shader_defs, + .. + } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); println!("defines: {:?}", shader_defs); let result = processor.preprocess(&WGSL, &shader_defs, true).unwrap(); assert_eq!(result.preprocessed_source, EXPECTED); @@ -1037,7 +1068,10 @@ bool: false "; let processor = Preprocessor::default(); - let PreprocessorMetaData { defines: shader_defs, .. } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); + let PreprocessorMetaData { + defines: shader_defs, + .. + } = processor.get_preprocessor_metadata(&WGSL, true).unwrap(); println!("defines: {:?}", shader_defs); let result = processor.preprocess(&WGSL, &shader_defs, true).unwrap(); assert_eq!(result.preprocessed_source, EXPECTED); diff --git a/src/compose/test.rs b/src/compose/test.rs index b6b05fb..6038ec5 100644 --- a/src/compose/test.rs +++ b/src/compose/test.rs @@ -12,8 +12,8 @@ mod test { }; use crate::compose::{ - ComposableModuleDescriptor, Composer, ImportDefinition, - NagaModuleDescriptor, ShaderDefValue, ShaderLanguage, ShaderType, + ComposableModuleDescriptor, Composer, ImportDefinition, NagaModuleDescriptor, + ShaderDefValue, ShaderLanguage, ShaderType, }; macro_rules! output_eq { @@ -897,11 +897,13 @@ mod test { ..Default::default() }) .unwrap(); - let module = composer.make_naga_module(NagaModuleDescriptor { - source: include_str!("tests/invalid_identifiers/top_valid.wgsl"), - file_path: "tests/invalid_identifiers/top_valid.wgsl", - ..Default::default() - }).unwrap(); + let module = composer + .make_naga_module(NagaModuleDescriptor { + source: include_str!("tests/invalid_identifiers/top_valid.wgsl"), + file_path: "tests/invalid_identifiers/top_valid.wgsl", + ..Default::default() + }) + .unwrap(); let info = naga::valid::Validator::new( naga::valid::ValidationFlags::all(), @@ -925,11 +927,14 @@ mod test { output_eq!(wgsl, "tests/expected/bad_identifiers.txt"); - composer.make_naga_module(NagaModuleDescriptor { - source: include_str!("tests/invalid_identifiers/top_invalid.wgsl"), - file_path: "tests/invalid_identifiers/top_invalid.wgsl", - ..Default::default() - }).err().unwrap(); + composer + .make_naga_module(NagaModuleDescriptor { + source: include_str!("tests/invalid_identifiers/top_invalid.wgsl"), + file_path: "tests/invalid_identifiers/top_invalid.wgsl", + ..Default::default() + }) + .err() + .unwrap(); } #[test] diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index f6984bc..7dabca8 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -1,102 +1,110 @@ -use std::collections::VecDeque; - -#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] -pub enum Token<'a> { - Identifier(&'a str, usize), - Other(char, usize), - Whitespace(&'a str, usize), -} - -impl<'a> Token<'a> { - pub fn pos(&self) -> usize { - match self { - Token::Identifier(_, pos) | - Token::Other(_, pos) | - Token::Whitespace(_, pos) => *pos - } - } - - pub fn identifier(&self) -> Option<&str> { - match self { - Token::Identifier(ident, _) => Some(ident), - _ => None, - } - } -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum TokenKind { - Identifier, - Whitespace, -} - -// a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens -// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters -pub struct Tokenizer<'a> { - tokens: VecDeque>, -} - -impl<'a> Tokenizer<'a> { - pub fn new(src: &'a str, emit_whitespace: bool) -> Self{ - let mut tokens = VecDeque::default(); - let mut current_token_start = 0; - let mut current_token = None; - - // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue - for (ix, char) in src.char_indices() { - if let Some(tok) = current_token { - match tok { - TokenKind::Identifier => { - if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { - continue; - } - tokens.push_back(Token::Identifier(&src[current_token_start..ix], current_token_start)); - } - TokenKind::Whitespace => { - if char.is_whitespace() { - continue; - } - tokens.push_back(Token::Whitespace(&src[current_token_start..ix], current_token_start)); - } - }; - - current_token_start = ix; - current_token = None; - } - - if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { - current_token = Some(TokenKind::Identifier); - current_token_start = ix; - } else if !char.is_whitespace() { - tokens.push_back(Token::Other(char, current_token_start)); - } else if char.is_whitespace() && emit_whitespace { - current_token = Some(TokenKind::Whitespace); - current_token_start = ix; - } - } - - if let Some(tok) = current_token { - match tok { - TokenKind::Identifier => { - tokens.push_back(Token::Identifier(&src[current_token_start..src.len()], current_token_start)); - } - TokenKind::Whitespace => { - tokens.push_back(Token::Whitespace(&src[current_token_start..src.len()], current_token_start)); - } - }; - } - - Self { - tokens, - } - } -} - -impl<'a> Iterator for Tokenizer<'a> { - type Item = Token<'a>; - - fn next(&mut self) -> Option { - let tok = self.tokens.pop_front(); - tok - } -} +use std::collections::VecDeque; + +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub enum Token<'a> { + Identifier(&'a str, usize), + Other(char, usize), + Whitespace(&'a str, usize), +} + +impl<'a> Token<'a> { + pub fn pos(&self) -> usize { + match self { + Token::Identifier(_, pos) | Token::Other(_, pos) | Token::Whitespace(_, pos) => *pos, + } + } + + pub fn identifier(&self) -> Option<&str> { + match self { + Token::Identifier(ident, _) => Some(ident), + _ => None, + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum TokenKind { + Identifier, + Whitespace, +} + +// a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens +// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters +pub struct Tokenizer<'a> { + tokens: VecDeque>, +} + +impl<'a> Tokenizer<'a> { + pub fn new(src: &'a str, emit_whitespace: bool) -> Self { + let mut tokens = VecDeque::default(); + let mut current_token_start = 0; + let mut current_token = None; + + // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue + for (ix, char) in src.char_indices() { + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { + continue; + } + tokens.push_back(Token::Identifier( + &src[current_token_start..ix], + current_token_start, + )); + } + TokenKind::Whitespace => { + if char.is_whitespace() { + continue; + } + tokens.push_back(Token::Whitespace( + &src[current_token_start..ix], + current_token_start, + )); + } + }; + + current_token_start = ix; + current_token = None; + } + + if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { + current_token = Some(TokenKind::Identifier); + current_token_start = ix; + } else if !char.is_whitespace() { + tokens.push_back(Token::Other(char, current_token_start)); + } else if char.is_whitespace() && emit_whitespace { + current_token = Some(TokenKind::Whitespace); + current_token_start = ix; + } + } + + if let Some(tok) = current_token { + match tok { + TokenKind::Identifier => { + tokens.push_back(Token::Identifier( + &src[current_token_start..src.len()], + current_token_start, + )); + } + TokenKind::Whitespace => { + tokens.push_back(Token::Whitespace( + &src[current_token_start..src.len()], + current_token_start, + )); + } + }; + } + + Self { tokens } + } +} + +impl<'a> Iterator for Tokenizer<'a> { + type Item = Token<'a>; + + fn next(&mut self) -> Option { + let tok = self.tokens.pop_front(); + tok + } +} From 0098baae0b3dd3797ff2e7a6b0c459e6d627bb6a Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 30 Jul 2023 22:56:48 +0100 Subject: [PATCH 06/30] clippy --- src/compose/parse_imports.rs | 2 +- src/compose/preprocess.rs | 12 +++++------- src/compose/tokenizer.rs | 3 +-- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 11639fe..2621634 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -94,7 +94,7 @@ pub fn parse_imports<'a>( tokens.next(); } - if !stack.is_empty() && !(is_deprecated_itemlist && stack.len() == 1) { + if !(stack.is_empty() || is_deprecated_itemlist && stack.len() == 1) { return Err(("missing close brace", input.len())); } diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index bba850e..1e62892 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -258,12 +258,10 @@ impl Preprocessor { } else if self .check_scope(shader_defs, &line, Some(&mut scope), offset)? .0 + || self.define_import_path_regex.captures(&line).is_some() + || self.define_shader_def_regex.captures(&line).is_some() { // ignore - } else if self.define_import_path_regex.captures(&line).is_some() { - // ignore - } else if self.define_shader_def_regex.captures(&line).is_some() { - // ignore } else if scope.active() { if self.import_regex.is_match(&line) { let mut import_lines = String::default(); @@ -295,7 +293,7 @@ impl Preprocessor { } else { let replaced_lines = [original_line, &line].map(|input| { let mut output = input.to_string(); - for capture in self.def_regex.captures_iter(&input) { + for capture in self.def_regex.captures_iter(input) { let def = capture.get(1).unwrap(); if let Some(def) = shader_defs.get(def.as_str()) { output = self @@ -304,7 +302,7 @@ impl Preprocessor { .to_string(); } } - for capture in self.def_regex_delimited.captures_iter(&input) { + for capture in self.def_regex_delimited.captures_iter(input) { let def = capture.get(1).unwrap(); if let Some(def) = shader_defs.get(def.as_str()) { output = self @@ -345,7 +343,7 @@ impl Preprocessor { final_string.push_str(&item_replaced_line); let diff = line.len().saturating_sub(item_replaced_line.len()); - final_string.extend(std::iter::repeat(" ").take(diff as usize)); + final_string.extend(std::iter::repeat(" ").take(diff)); offset += original_line.len() + 1; output = true; } diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index 7dabca8..baeb4a4 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -104,7 +104,6 @@ impl<'a> Iterator for Tokenizer<'a> { type Item = Token<'a>; fn next(&mut self) -> Option { - let tok = self.tokens.pop_front(); - tok + self.tokens.pop_front() } } From 9ec6520e036408b51acac3e8debc8446c8339004 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 31 Jul 2023 17:24:25 +0100 Subject: [PATCH 07/30] fix toplevel err source after preprocess --- src/compose/mod.rs | 59 +++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index e337a2e..42be340 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -819,6 +819,8 @@ impl Composer { shader_defs: &HashMap, create_headers: bool, demote_entrypoints: bool, + source: &str, + imports: Vec, ) -> Result { let wrap_err = |inner: ComposerErrorInner| -> ComposerError { ComposerError { @@ -831,18 +833,6 @@ impl Composer { } }; - let PreprocessOutput { - preprocessed_source: source, - imports, - } = self - .preprocessor - .preprocess( - &module_definition.sanitized_source, - shader_defs, - self.validate, - ) - .map_err(wrap_err)?; - let mut imports: Vec<_> = imports .into_iter() .map(|import_with_offset| import_with_offset.definition) @@ -1277,7 +1267,10 @@ impl Composer { module_set: &ComposableModuleDefinition, shader_defs: &HashMap, ) -> Result { - let imports = self + let PreprocessOutput { + preprocessed_source, + imports, + } = self .preprocessor .preprocess(&module_set.sanitized_source, shader_defs, self.validate) .map_err(|inner| ComposerError { @@ -1287,8 +1280,7 @@ impl Composer { offset: 0, defs: shader_defs.clone(), }, - })? - .imports; + })?; self.ensure_imports(imports.iter().map(|import| &import.definition), shader_defs)?; self.ensure_imports(&module_set.additional_imports, shader_defs)?; @@ -1299,6 +1291,8 @@ impl Composer { shader_defs, true, true, + &preprocessed_source, + imports, ) } @@ -1621,13 +1615,36 @@ impl Composer { modules: Default::default(), }; + let PreprocessOutput { + preprocessed_source, + imports, + } = self + .preprocessor + .preprocess(&sanitized_source, &shader_defs, self.validate) + .map_err(|inner| ComposerError { + inner, + source: ErrSource::Constructing { + path: file_path.to_owned(), + source: sanitized_source, + offset: 0, + }, + })?; + let composable = self - .create_composable_module(&definition, String::from(""), &shader_defs, false, false) + .create_composable_module( + &definition, + String::from(""), + &shader_defs, + false, + false, + &preprocessed_source, + imports, + ) .map_err(|e| ComposerError { inner: e.inner, source: ErrSource::Constructing { path: definition.file_path.to_owned(), - source: definition.sanitized_source.to_owned(), + source: preprocessed_source.clone(), offset: e.source.offset(), }, })?; @@ -1686,7 +1703,7 @@ impl Composer { inner: e.into(), source: ErrSource::Constructing { path: file_path.to_owned(), - source: sanitized_source.clone(), + source: preprocessed_source.clone(), offset: composable.start_offset, }, })?; @@ -1699,7 +1716,7 @@ impl Composer { inner: e.into(), source: ErrSource::Constructing { path: file_path.to_owned(), - source: sanitized_source.clone(), + source: preprocessed_source.clone(), offset: composable.start_offset, }, })?; @@ -1720,7 +1737,7 @@ impl Composer { match module_index { 0 => ErrSource::Constructing { path: file_path.to_owned(), - source: definition.sanitized_source, + source: preprocessed_source.clone(), offset: composable.start_offset, }, _ => { @@ -1743,7 +1760,7 @@ impl Composer { } None => ErrSource::Constructing { path: file_path.to_owned(), - source: sanitized_source, + source: preprocessed_source.clone(), offset: composable.start_offset, }, }; From 21ead5736ef342f8dd431fff4620289f06317646 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 31 Jul 2023 17:34:25 +0100 Subject: [PATCH 08/30] disallow leading/trailing `:`s on ident tokens --- src/compose/tokenizer.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index baeb4a4..23ccb5e 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -29,7 +29,8 @@ enum TokenKind { } // a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens -// unicode XID rules apply, except that double quotes (`"`) and colons (`:`) are also allowed as identifier characters +// unicode XID rules apply, except that double quotes (`"`) are also allowed as identifier characters, +// and colons (`:`) are allowed except in the initial and final positions pub struct Tokenizer<'a> { tokens: VecDeque>, } @@ -48,10 +49,21 @@ impl<'a> Tokenizer<'a> { if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { continue; } + + // separate trailing ':'s + let mut actual_ix = ix; + while src[current_token_start..actual_ix].ends_with(':') { + actual_ix -= 1; + } + tokens.push_back(Token::Identifier( - &src[current_token_start..ix], + &src[current_token_start..actual_ix], current_token_start, )); + + for trailing_ix in actual_ix..ix { + tokens.push_back(Token::Other(':', trailing_ix)); + } } TokenKind::Whitespace => { if char.is_whitespace() { @@ -68,7 +80,7 @@ impl<'a> Tokenizer<'a> { current_token = None; } - if unicode_ident::is_xid_start(char) || char == '"' || char == ':' { + if unicode_ident::is_xid_start(char) || char == '"' { current_token = Some(TokenKind::Identifier); current_token_start = ix; } else if !char.is_whitespace() { From 23e7676c0348f63fe2e5ff040f348a747dcbeeff Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 31 Jul 2023 17:34:32 +0100 Subject: [PATCH 09/30] ci --- src/compose/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 42be340..4e5a4a4 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -812,6 +812,7 @@ impl Composer { // - build the naga IR (against headers) // - record any types/vars/constants/functions that are defined within this module // - build headers for each supported language + #[allow(clippy::too_many_arguments)] fn create_composable_module( &mut self, module_definition: &ComposableModuleDefinition, @@ -849,7 +850,7 @@ impl Composer { let mut virtual_functions: HashSet = Default::default(); let source = self .virtual_fn_regex - .replace_all(&source, |cap: ®ex::Captures| { + .replace_all(source, |cap: ®ex::Captures| { let target_function = cap.get(2).unwrap().as_str().to_owned(); let replacement_str = format!( From 3781ad9e67310100d42529f802395ae3c463c9ab Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 31 Jul 2023 17:43:51 +0100 Subject: [PATCH 10/30] allow trailing ':'s for imports --- src/compose/parse_imports.rs | 4 ++-- src/compose/tokenizer.rs | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 2621634..ca3911e 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -9,7 +9,7 @@ pub fn parse_imports<'a>( input: &'a str, declared_imports: &mut HashMap>, ) -> Result<(), (&'a str, usize)> { - let mut tokens = Tokenizer::new(input, false).peekable(); + let mut tokens = Tokenizer::new(input, false, true).peekable(); match tokens.next() { Some(Token::Other('#', _)) => (), @@ -108,7 +108,7 @@ pub fn substitute_identifiers( used_imports: &mut HashMap, allow_ambiguous: bool, ) -> Result { - let tokens = Tokenizer::new(input, true); + let tokens = Tokenizer::new(input, true, false); let mut output = String::with_capacity(input.len()); let mut in_substitution_position = true; diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index 23ccb5e..4c5d744 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -36,7 +36,7 @@ pub struct Tokenizer<'a> { } impl<'a> Tokenizer<'a> { - pub fn new(src: &'a str, emit_whitespace: bool) -> Self { + pub fn new(src: &'a str, emit_whitespace: bool, allow_trailing_colons: bool) -> Self { let mut tokens = VecDeque::default(); let mut current_token_start = 0; let mut current_token = None; @@ -52,8 +52,11 @@ impl<'a> Tokenizer<'a> { // separate trailing ':'s let mut actual_ix = ix; - while src[current_token_start..actual_ix].ends_with(':') { - actual_ix -= 1; + + if !allow_trailing_colons { + while src[current_token_start..actual_ix].ends_with(':') { + actual_ix -= 1; + } } tokens.push_back(Token::Identifier( From 61ef532821e4a24bf1099a1fef8130d0093af355 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 31 Jul 2023 19:08:03 +0100 Subject: [PATCH 11/30] better tokenizer - handle quoted strings as part of an identifier - allow only double colons as part of an identifier --- src/compose/mod.rs | 2 +- src/compose/parse_imports.rs | 26 +++++++- src/compose/test.rs | 59 ++++++++++++++++++- .../tests/bevy_path_imports/skill.wgsl | 31 ++++++++++ src/compose/tokenizer.rs | 43 +++++++------- 5 files changed, 136 insertions(+), 25 deletions(-) create mode 100644 src/compose/tests/bevy_path_imports/skill.wgsl diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 4e5a4a4..0347086 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -292,7 +292,7 @@ impl ComposableModuleDefinition { } } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ImportDefinition { pub import: String, pub items: Vec, diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index ca3911e..e2aa2ae 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -9,7 +9,7 @@ pub fn parse_imports<'a>( input: &'a str, declared_imports: &mut HashMap>, ) -> Result<(), (&'a str, usize)> { - let mut tokens = Tokenizer::new(input, false, true).peekable(); + let mut tokens = Tokenizer::new(input, false).peekable(); match tokens.next() { Some(Token::Other('#', _)) => (), @@ -108,7 +108,7 @@ pub fn substitute_identifiers( used_imports: &mut HashMap, allow_ambiguous: bool, ) -> Result { - let tokens = Tokenizer::new(input, true, false); + let tokens = Tokenizer::new(input, true); let mut output = String::with_capacity(input.len()); let mut in_substitution_position = true; @@ -305,6 +305,23 @@ fn import_tokens() { ])) ); + let input = r#" + #import "path//with\ all sorts of .stuff"::{a, b} + "#; + assert_eq!( + test_parse(input), + Ok(HashMap::from_iter([ + ( + "a".to_owned(), + vec!(r#""path//with\ all sorts of .stuff"::a"#.to_owned()) + ), + ( + "b".to_owned(), + vec!(r#""path//with\ all sorts of .stuff"::b"#.to_owned()) + ), + ])) + ); + let input = r" #import a::b::{ "; @@ -324,4 +341,9 @@ fn import_tokens() { #import a::b{{c,d}} "; assert!(test_parse(input).is_err()); + + let input = r" + #import a:b + "; + assert!(test_parse(input).is_err()); } diff --git a/src/compose/test.rs b/src/compose/test.rs index 6038ec5..d15a843 100644 --- a/src/compose/test.rs +++ b/src/compose/test.rs @@ -12,8 +12,8 @@ mod test { }; use crate::compose::{ - ComposableModuleDescriptor, Composer, ImportDefinition, NagaModuleDescriptor, - ShaderDefValue, ShaderLanguage, ShaderType, + get_preprocessor_data, ComposableModuleDescriptor, Composer, ImportDefinition, + NagaModuleDescriptor, ShaderDefValue, ShaderLanguage, ShaderType, }; macro_rules! output_eq { @@ -1144,6 +1144,61 @@ mod test { assert_eq!(test_shader(&mut composer), 36.0); } + #[test] + fn test_bevy_path_imports() { + let (_, mut imports, _) = + get_preprocessor_data(include_str!("tests/bevy_path_imports/skill.wgsl")); + imports.iter_mut().for_each(|import| { + import.items.sort(); + }); + imports.sort_by(|a, b| a.import.cmp(&b.import)); + assert_eq!( + imports, + vec![ + ImportDefinition { + import: "\"shaders/skills/hit.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/lightning.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/lightning_ring.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/magic_arrow.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/orb.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/railgun_trail.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/shared.wgsl\"".to_owned(), + items: vec![ + "Vertex".to_owned(), + "VertexOutput".to_owned(), + "VertexOutput".to_owned(), + ], + }, + ImportDefinition { + import: "\"shaders/skills/slash.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ImportDefinition { + import: "\"shaders/skills/sound.wgsl\"".to_owned(), + items: vec!["frag".to_owned(), "vert".to_owned(),], + }, + ] + ); + } + // actually run a shader and extract the result // needs the composer to contain a module called "test_module", with a function called "entry_point" returning an f32. fn test_shader(composer: &mut Composer) -> f32 { diff --git a/src/compose/tests/bevy_path_imports/skill.wgsl b/src/compose/tests/bevy_path_imports/skill.wgsl new file mode 100644 index 0000000..04a9139 --- /dev/null +++ b/src/compose/tests/bevy_path_imports/skill.wgsl @@ -0,0 +1,31 @@ +#import "shaders/skills/shared.wgsl" Vertex, VertexOutput + +#if EFFECT_ID == 0 + #import "shaders/skills/sound.wgsl" frag, vert +#else if EFFECT_ID == 1 + #import "shaders/skills/orb.wgsl" frag, vert +#else if EFFECT_ID == 2 + #import "shaders/skills/slash.wgsl" frag, vert +#else if EFFECT_ID == 3 + #import "shaders/skills/railgun_trail.wgsl" frag, vert +#else if EFFECT_ID == 4 + #import "shaders/skills/magic_arrow.wgsl" frag, vert +#else if EFFECT_ID == 5 + #import "shaders/skills/hit.wgsl" frag, vert +#else if EFFECT_ID == 6 + #import "shaders/skills/lightning_ring.wgsl" frag, vert +#else if EFFECT_ID == 7 + #import "shaders/skills/lightning.wgsl" frag, vert +#endif + +#import something_unused + +@fragment +fn fragment(in: VertexOutput) -> @location(0) vec4 { + return frag(in); +} + +@vertex +fn vertex(vertex: Vertex) -> VertexOutput { + return vert(vertex); +} diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index 4c5d744..107f09d 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -29,44 +29,47 @@ enum TokenKind { } // a basic tokenizer that separates identifiers from non-identifiers, and optionally returns whitespace tokens -// unicode XID rules apply, except that double quotes (`"`) are also allowed as identifier characters, -// and colons (`:`) are allowed except in the initial and final positions +// unicode XID rules apply, except that additional characters '"' and '::' (sequences of two colons) are allowed in identifiers. +// quotes treat any further chars until the next quote as part of the identifier. +// note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue pub struct Tokenizer<'a> { tokens: VecDeque>, } impl<'a> Tokenizer<'a> { - pub fn new(src: &'a str, emit_whitespace: bool, allow_trailing_colons: bool) -> Self { + pub fn new(src: &'a str, emit_whitespace: bool) -> Self { let mut tokens = VecDeque::default(); let mut current_token_start = 0; let mut current_token = None; + let mut quoted_token = false; + + let mut chars = src.char_indices().peekable(); + + while let Some((ix, char)) = chars.next() { + if char == '"' { + quoted_token = !quoted_token; + if !quoted_token { + continue; + } + } - // note we don't support non-USV identifiers like ๐Ÿ‘ฉโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ง which is apparently in XID_continue - for (ix, char) in src.char_indices() { if let Some(tok) = current_token { match tok { TokenKind::Identifier => { - if unicode_ident::is_xid_continue(char) || char == '"' || char == ':' { + // accept anything within quotes, or XID_continues + if quoted_token || unicode_ident::is_xid_continue(char) { continue; } - - // separate trailing ':'s - let mut actual_ix = ix; - - if !allow_trailing_colons { - while src[current_token_start..actual_ix].ends_with(':') { - actual_ix -= 1; - } + // accept `::` + if char == ':' && chars.peek() == Some(&(ix + 1, ':')) { + chars.next(); + continue; } tokens.push_back(Token::Identifier( - &src[current_token_start..actual_ix], + &src[current_token_start..ix], current_token_start, )); - - for trailing_ix in actual_ix..ix { - tokens.push_back(Token::Other(':', trailing_ix)); - } } TokenKind::Whitespace => { if char.is_whitespace() { @@ -83,7 +86,7 @@ impl<'a> Tokenizer<'a> { current_token = None; } - if unicode_ident::is_xid_start(char) || char == '"' { + if quoted_token || unicode_ident::is_xid_start(char) { current_token = Some(TokenKind::Identifier); current_token_start = ix; } else if !char.is_whitespace() { From 9d8b9ce1fa5d250186e8336af6d987e8a9e324e6 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Thu, 3 Aug 2023 23:59:12 +0100 Subject: [PATCH 12/30] remove get_preprocessor_data unwrap() --- src/compose/mod.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 0347086..1018dac 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -1789,20 +1789,23 @@ pub fn get_preprocessor_data( Vec, HashMap, ) { - let PreprocessorMetaData { + if let Ok(PreprocessorMetaData { name, imports, defines, .. - } = PREPROCESSOR - .get_preprocessor_metadata(source, true) - .unwrap(); - ( - name, - imports - .into_iter() - .map(|import_with_offset| import_with_offset.definition) - .collect(), - defines, - ) + }) = PREPROCESSOR.get_preprocessor_metadata(source, true) + { + ( + name, + imports + .into_iter() + .map(|import_with_offset| import_with_offset.definition) + .collect(), + defines, + ) + } else { + // if errors occur we return nothing; the actual error will be displayed when the caller attempts to use the shader + Default::default() + } } From 343d95f7f7d5c9d683b4062c9bb1fd9cebc0cb4a Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Thu, 3 Aug 2023 23:59:41 +0100 Subject: [PATCH 13/30] fix Token::Other offset --- src/compose/tokenizer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compose/tokenizer.rs b/src/compose/tokenizer.rs index 107f09d..b57577c 100644 --- a/src/compose/tokenizer.rs +++ b/src/compose/tokenizer.rs @@ -90,7 +90,7 @@ impl<'a> Tokenizer<'a> { current_token = Some(TokenKind::Identifier); current_token_start = ix; } else if !char.is_whitespace() { - tokens.push_back(Token::Other(char, current_token_start)); + tokens.push_back(Token::Other(char, ix)); } else if char.is_whitespace() && emit_whitespace { current_token = Some(TokenKind::Whitespace); current_token_start = ix; From e4fa468613461617205472e6dc063b043e79621c Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Fri, 4 Aug 2023 00:00:01 +0100 Subject: [PATCH 14/30] fix reported offset for import errors --- src/compose/preprocess.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 1e62892..3b0e525 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -266,6 +266,7 @@ impl Preprocessor { if self.import_regex.is_match(&line) { let mut import_lines = String::default(); let mut open_count = 0; + let initial_offset = offset; loop { // output spaces for removed lines to keep spans consistent (errors report against substituted_source, which is not preprocessed) @@ -276,6 +277,7 @@ impl Preprocessor { open_count = open_count.saturating_sub(line.match_indices('}').count()); import_lines.push_str(&line); + import_lines.push('\n'); if open_count == 0 || lines.peek().is_none() { break; @@ -285,8 +287,8 @@ impl Preprocessor { } parse_imports(import_lines.as_str(), &mut declared_imports).map_err( - |(err, offset)| { - ComposerErrorInner::ImportParseError(err.to_owned(), offset) + |(err, line_offset)| { + ComposerErrorInner::ImportParseError(err.to_owned(), initial_offset + line_offset) }, )?; output = true; @@ -399,12 +401,14 @@ impl Preprocessor { } else if self.import_regex.is_match(&line) { let mut import_lines = String::default(); let mut open_count = 0; + let initial_offset = offset; loop { open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); import_lines.push_str(&line); + import_lines.push('\n'); if open_count == 0 || lines.peek().is_none() { break; @@ -417,7 +421,7 @@ impl Preprocessor { } parse_imports(import_lines.as_str(), &mut declared_imports).map_err( - |(err, offset)| ComposerErrorInner::ImportParseError(err.to_owned(), offset), + |(err, line_offset)| ComposerErrorInner::ImportParseError(err.to_owned(), initial_offset + line_offset), )?; } else if let Some(cap) = self.define_import_path_regex.captures(&line) { name = Some(cap.get(1).unwrap().as_str().to_string()); From e3b2aaed1132d6f2245716b25f4feba14e2efa80 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Fri, 4 Aug 2023 00:00:16 +0100 Subject: [PATCH 15/30] allow trailing semicolon for import defs --- src/compose/parse_imports.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index e2aa2ae..98be849 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -60,7 +60,7 @@ pub fn parse_imports<'a>( current = String::default(); as_name = None; } - Some(Token::Other(',', _)) | Some(Token::Other('}', _)) | None => { + Some(Token::Other(',', _)) | Some(Token::Other('}', _)) | Some(Token::Other('\n', _)) | None => { if !current.is_empty() { let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| { current @@ -87,6 +87,12 @@ pub fn parse_imports<'a>( break; } } + Some(Token::Other(';', _)) => { + tokens.next(); + if let Some(token) = tokens.peek() { + return Err(("unexpected token after ';'", token.pos())); + } + } Some(Token::Other(_, pos)) => return Err(("unexpected token", *pos)), Some(Token::Whitespace(..)) => unreachable!(), } From 9043aace58cc902c9267752e3fbd761c0ab5c951 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Fri, 4 Aug 2023 00:00:32 +0100 Subject: [PATCH 16/30] ci --- src/compose/parse_imports.rs | 5 ++++- src/compose/preprocess.rs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 98be849..f11c5e1 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -60,7 +60,10 @@ pub fn parse_imports<'a>( current = String::default(); as_name = None; } - Some(Token::Other(',', _)) | Some(Token::Other('}', _)) | Some(Token::Other('\n', _)) | None => { + Some(Token::Other(',', _)) + | Some(Token::Other('}', _)) + | Some(Token::Other('\n', _)) + | None => { if !current.is_empty() { let used_name = as_name.map(ToString::to_string).unwrap_or_else(|| { current diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 3b0e525..78fc7c3 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -288,7 +288,10 @@ impl Preprocessor { parse_imports(import_lines.as_str(), &mut declared_imports).map_err( |(err, line_offset)| { - ComposerErrorInner::ImportParseError(err.to_owned(), initial_offset + line_offset) + ComposerErrorInner::ImportParseError( + err.to_owned(), + initial_offset + line_offset, + ) }, )?; output = true; @@ -421,7 +424,12 @@ impl Preprocessor { } parse_imports(import_lines.as_str(), &mut declared_imports).map_err( - |(err, line_offset)| ComposerErrorInner::ImportParseError(err.to_owned(), initial_offset + line_offset), + |(err, line_offset)| { + ComposerErrorInner::ImportParseError( + err.to_owned(), + initial_offset + line_offset, + ) + }, )?; } else if let Some(cap) = self.define_import_path_regex.captures(&line) { name = Some(cap.get(1).unwrap().as_str().to_string()); From 0cb845f393e637f82b80983f41b55dfff857ab3c Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Fri, 4 Aug 2023 10:45:04 +0100 Subject: [PATCH 17/30] update readme --- readme.md | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/readme.md b/readme.md index 2a2ef0c..1ec57f6 100644 --- a/readme.md +++ b/readme.md @@ -32,15 +32,17 @@ fn my_func() -> f32 { } ``` -shaders can then import the module with an `#import` directive (with an optional `as` name). at point of use, imported items must be qualified: +alternatively the module name can be specified as an argument to `Composer::add_composable_module`. + +shaders can then import the module with an `#import` directive (with an optional `as` name) : ```wgsl -#import my_module -#import my_other_module as Mod2 +#import my_module; +#import my_other_module as mod2; fn main() -> f32 { let x = my_module::my_func(); - let y = Mod2::my_other_func(); + let y = mod2::my_other_func(); return x*y; } ``` @@ -48,18 +50,33 @@ fn main() -> f32 { or import a comma-separated list of individual items : ```wgsl -#import my_module my_func, my_const +#import my_module::{my_func, my_const} fn main() -> f32 { return my_func(my_const); } ``` +Some rust-style import syntax is supported, and items can be directly imported using the fully qualified item name : + +```wgsl +#import my_package::{ + first_module::{item_one as item, item_two}, + second_module::submodule, +} + +fn main() -> f32 { + return item + item_two + submodule::subitem + my_package::third_module::item; +} +``` + +`module::self` and `module::*` are not currently supported. + imports can be nested - modules may import other modules, but not recursively. when a new module is added, all its `#import`s must already have been added. the same module can be imported multiple times by different modules in the import tree. there is no overlap of namespaces, so the same function names (or type, constant, or variable names) may be used in different modules. -note: when importing an item with the `#import module item` directive, the final shader will include the required dependencies (bindings, globals, consts, other functions) of the imported item, but will not include the rest of the imported module. it will however still include all of any modules imported by the imported module. this is probably not desired in general and may be fixed in a future version. currently for a more complete culling of unused dependencies the `prune` module can be used. +note: the final shader will include the required dependencies (bindings, globals, consts, other functions) of any imported items that are used, but will not include the rest of the imported module. ## overriding functions @@ -79,6 +96,8 @@ override fn Lighting::point_light (world_position: vec3) -> vec3 { } ``` +overrides must either be declared in the top-level shader, or the module containing the override must be imported as an `additional_import` in a `Composer::add_composable_module` or `Composer::make_naga_module` call. using `#import` to import a module with overrides will not work due to tree-shaking. + override function definitions cause *all* calls to the original function in the entire shader scope to be replaced by calls to the new function, with the exception of calls within the override function itself. the function signature of the override must match the base function. From 38eb7af5b6006215a4933f13d690b3e8cb6633de Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:17:40 +0100 Subject: [PATCH 18/30] don't sub isolated idents for quoted fullpaths --- src/compose/parse_imports.rs | 9 +++++ src/compose/test.rs | 40 +++++++++++++++++++ .../expected/test_quoted_import_dup_name.txt | 14 +++++++ src/compose/tests/quoted_dup/mod.wgsl | 5 +++ src/compose/tests/quoted_dup/top.wgsl | 9 +++++ 5 files changed, 77 insertions(+) create mode 100644 src/compose/tests/expected/test_quoted_import_dup_name.txt create mode 100644 src/compose/tests/quoted_dup/mod.wgsl create mode 100644 src/compose/tests/quoted_dup/top.wgsl diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index f11c5e1..a9464b5 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -156,7 +156,16 @@ pub fn substitute_identifiers( .push(item.to_owned()); output.push_str(item); output.push_str(&Composer::decorate(module)); + } else if full_path.find('"').is_some() { + // we don't want to replace local variables that shadow quoted module imports with the + // quoted name as that won't compile. + // since quoted items always refer to modules, we can just emit the original ident + // in this case + output.push_str(ident); } else { + // if there are no quotes we do the replacement. this means that individually imported + // items can be used, and any shadowing local variables get harmlessly renamed. + // TODO: it can lead to weird errors, but such is life output.push_str(&full_path); } } diff --git a/src/compose/test.rs b/src/compose/test.rs index d15a843..711332b 100644 --- a/src/compose/test.rs +++ b/src/compose/test.rs @@ -1199,6 +1199,46 @@ mod test { ); } + #[test] + fn test_quoted_import_dup_name() { + let mut composer = Composer::default(); + + composer + .add_composable_module(ComposableModuleDescriptor { + source: include_str!("tests/quoted_dup/mod.wgsl"), + file_path: "tests/quoted_dup/mod.wgsl", + ..Default::default() + }) + .unwrap(); + + let module = composer + .make_naga_module(NagaModuleDescriptor { + source: include_str!("tests/quoted_dup/top.wgsl"), + file_path: "tests/quoted_dup/top.wgsl", + ..Default::default() + }) + .unwrap(); + + let info = naga::valid::Validator::new( + naga::valid::ValidationFlags::all(), + naga::valid::Capabilities::default(), + ) + .validate(&module) + .unwrap(); + let wgsl = naga::back::wgsl::write_string( + &module, + &info, + naga::back::wgsl::WriterFlags::EXPLICIT_TYPES, + ) + .unwrap(); + + // let mut f = std::fs::File::create("test_quoted_import_dup_name.txt").unwrap(); + // f.write_all(wgsl.as_bytes()).unwrap(); + // drop(f); + + output_eq!(wgsl, "tests/expected/test_quoted_import_dup_name.txt"); + } + // actually run a shader and extract the result // needs the composer to contain a module called "test_module", with a function called "entry_point" returning an f32. fn test_shader(composer: &mut Composer) -> f32 { diff --git a/src/compose/tests/expected/test_quoted_import_dup_name.txt b/src/compose/tests/expected/test_quoted_import_dup_name.txt new file mode 100644 index 0000000..ce3b573 --- /dev/null +++ b/src/compose/tests/expected/test_quoted_import_dup_name.txt @@ -0,0 +1,14 @@ +fn fooX_naga_oil_mod_XEJYXK33UMVSF63LPMR2WYZJCX() -> f32 { + return 3.0; +} + +fn myfunc(foo: u32) -> f32 { + return (f32(foo) * 2.0); +} + +fn main() -> f32 { + let _e1: f32 = myfunc(1u); + let _e2: f32 = fooX_naga_oil_mod_XEJYXK33UMVSF63LPMR2WYZJCX(); + return (_e1 + _e2); +} + diff --git a/src/compose/tests/quoted_dup/mod.wgsl b/src/compose/tests/quoted_dup/mod.wgsl new file mode 100644 index 0000000..d7cce94 --- /dev/null +++ b/src/compose/tests/quoted_dup/mod.wgsl @@ -0,0 +1,5 @@ +#define_import_path "quoted_module" + +fn foo() -> f32 { + return 3.0; +} \ No newline at end of file diff --git a/src/compose/tests/quoted_dup/top.wgsl b/src/compose/tests/quoted_dup/top.wgsl new file mode 100644 index 0000000..0ff6e24 --- /dev/null +++ b/src/compose/tests/quoted_dup/top.wgsl @@ -0,0 +1,9 @@ +#import "quoted_module" as foo; + +fn myfunc(foo: u32) -> f32 { + return f32(foo) * 2.0; +} + +fn main() -> f32 { + return myfunc(1u) + foo::foo(); +} \ No newline at end of file From a7716b328c0bd39728978ea673531fc9c0b35e9a Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 11 Sep 2023 00:51:16 +0100 Subject: [PATCH 19/30] fix test output --- src/compose/tests/expected/bad_identifiers.txt | 1 - src/compose/tests/expected/dup_import.txt | 2 +- src/compose/tests/expected/import_in_decl.txt | 2 +- src/compose/tests/expected/use_shared_global.txt | 8 ++++---- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/compose/tests/expected/bad_identifiers.txt b/src/compose/tests/expected/bad_identifiers.txt index 164f345..a67db37 100644 --- a/src/compose/tests/expected/bad_identifiers.txt +++ b/src/compose/tests/expected/bad_identifiers.txt @@ -6,7 +6,6 @@ - d.fine = 3.0; e.fine_member = 4.0; fine: f32, diff --git a/src/compose/tests/expected/dup_import.txt b/src/compose/tests/expected/dup_import.txt index ebc0cae..822ce70 100644 --- a/src/compose/tests/expected/dup_import.txt +++ b/src/compose/tests/expected/dup_import.txt @@ -7,7 +7,7 @@ return (PIX_naga_oil_mod_XMNXW443UOMX * 1.0); return (PIX_naga_oil_mod_XMNXW443UOMX * 2.0); return (_e0 * _e1); -const PIX_naga_oil_mod_XMNXW443UOMX: f32 = 3.0999999046325684; +const PIX_naga_oil_mod_XMNXW443UOMX: f32 = 3.1; fn fX_naga_oil_mod_XMEX() -> f32 { fn fX_naga_oil_mod_XMIX() -> f32 { fn main() -> f32 { diff --git a/src/compose/tests/expected/import_in_decl.txt b/src/compose/tests/expected/import_in_decl.txt index 4ecba51..abc899f 100644 --- a/src/compose/tests/expected/import_in_decl.txt +++ b/src/compose/tests/expected/import_in_decl.txt @@ -1,6 +1,6 @@ const XX_naga_oil_mod_XMNXW443UOMX: u32 = 1u; -var arrX_naga_oil_mod_XMJUW4ZAX: array; +var arrX_naga_oil_mod_XMJUW4ZAX: array; fn main() -> f32 { let _e2: u32 = arrX_naga_oil_mod_XMJUW4ZAX[0]; diff --git a/src/compose/tests/expected/use_shared_global.txt b/src/compose/tests/expected/use_shared_global.txt index 54d95c9..f4a1263 100644 --- a/src/compose/tests/expected/use_shared_global.txt +++ b/src/compose/tests/expected/use_shared_global.txt @@ -1,15 +1,15 @@ -var _naga_oil_mod_NVXWI_membera: f32 = 0.0; +var aX_naga_oil_mod_XNVXWIX: f32 = 0.0; fn add() { - let _e2: f32 = _naga_oil_mod_NVXWI_membera; - _naga_oil_mod_NVXWI_membera = (_e2 + 1.0); + let _e2: f32 = aX_naga_oil_mod_XNVXWIX; + aX_naga_oil_mod_XNVXWIX = (_e2 + 1.0); return; } fn main() -> f32 { add(); add(); - let _e1: f32 = _naga_oil_mod_NVXWI_membera; + let _e1: f32 = aX_naga_oil_mod_XNVXWIX; return _e1; } From a6970a08d1f6a2008e00d918b076d9e2c4985154 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 11 Sep 2023 00:51:31 +0100 Subject: [PATCH 20/30] fmt --- src/compose/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 49e6a48..733fcef 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -144,8 +144,8 @@ pub mod error; pub mod parse_imports; pub mod preprocess; mod test; -pub mod util; pub mod tokenizer; +pub mod util; #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug, Default)] pub enum ShaderLanguage { From 2e0b92358cb5a8fb8b59d26f2bf07ff6bc2e7be4 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 11 Sep 2023 00:52:47 +0100 Subject: [PATCH 21/30] clippy --- src/compose/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 733fcef..2185db9 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -1562,7 +1562,7 @@ impl Composer { offset: 0, }, })?; - shader_defs.extend(defines.into_iter()); + shader_defs.extend(defines); let name = name.unwrap_or_default(); From 8ef675c6a540931b2d77ebc8c8a67ff60d82b02f Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sat, 23 Sep 2023 20:13:03 +0100 Subject: [PATCH 22/30] fix atomics test output --- src/compose/tests/expected/atomics.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/compose/tests/expected/atomics.txt b/src/compose/tests/expected/atomics.txt index 051f2e4..760d5f0 100644 --- a/src/compose/tests/expected/atomics.txt +++ b/src/compose/tests/expected/atomics.txt @@ -3,31 +3,31 @@ struct gen___atomic_compare_exchange_resultUint4_ { exchanged: bool, } -var _naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom: atomic; +var atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX: atomic; -fn _naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberentry_point() -> f32 { +fn entry_pointX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX() -> f32 { var y: u32; - atomicStore((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 1u); - let _e3: u32 = atomicLoad((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom)); + atomicStore((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 1u); + let _e3: u32 = atomicLoad((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX)); y = _e3; - let _e7: u32 = atomicAdd((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 2u); + let _e7: u32 = atomicAdd((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 2u); let _e8: u32 = y; y = (_e8 + _e7); - let _e12: u32 = atomicSub((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 1u); + let _e12: u32 = atomicSub((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 1u); let _e13: u32 = y; y = (_e13 + _e12); - let _e17: u32 = atomicMax((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 5u); + let _e17: u32 = atomicMax((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 5u); let _e18: u32 = y; y = (_e18 + _e17); - let _e22: u32 = atomicMin((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 4u); + let _e22: u32 = atomicMin((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 4u); let _e23: u32 = y; y = (_e23 + _e22); let _e25: u32 = y; - let _e27: u32 = atomicExchange((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), _e25); + let _e27: u32 = atomicExchange((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), _e25); let _e28: u32 = y; y = (_e28 + _e27); - let _e33: gen___atomic_compare_exchange_resultUint4_ = atomicCompareExchangeWeak((&_naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberatom), 12u, 0u); + let _e33: gen___atomic_compare_exchange_resultUint4_ = atomicCompareExchangeWeak((&atomX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX), 12u, 0u); if _e33.exchanged { let _e36: u32 = y; y = (_e36 + _e33.old_value); @@ -37,7 +37,7 @@ fn _naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberentry_point() -> f32 { } fn main() -> f32 { - let _e0: f32 = _naga_oil_mod_ORSXG5C7NVXWI5LMMU_memberentry_point(); + let _e0: f32 = entry_pointX_naga_oil_mod_XORSXG5C7NVXWI5LMMUX(); return _e0; } From cce4144b70d25c32a7839c8dbdce76ca3355b559 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:28:44 +0100 Subject: [PATCH 23/30] avoid unused warning --- src/compose/mod.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index eb55862..5c5ef06 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -828,17 +828,6 @@ impl Composer { source: &str, imports: Vec, ) -> Result { - let wrap_err = |inner: ComposerErrorInner| -> ComposerError { - ComposerError { - inner, - source: ErrSource::Module { - name: module_definition.name.to_owned(), - offset: 0, - defs: shader_defs.clone(), - }, - } - }; - let mut imports: Vec<_> = imports .into_iter() .map(|import_with_offset| import_with_offset.definition) @@ -884,6 +873,17 @@ impl Composer { #[cfg(not(feature = "override_any"))] { + let wrap_err = |inner: ComposerErrorInner| -> ComposerError { + ComposerError { + inner, + source: ErrSource::Module { + name: module_definition.name.to_owned(), + offset: 0, + defs: shader_defs.clone(), + }, + } + }; + // ensure overrides are applied to virtual functions let raw_module_name = Self::decode(&target_module); let module_set = self.module_sets.get(&raw_module_name); From da0891d09808c2449feddf62c4b74cde9946ca01 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:35:56 +0100 Subject: [PATCH 24/30] warn on deprecated itemlist imports --- Cargo.toml | 3 ++- src/compose/parse_imports.rs | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 39d5f21..9feb826 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,11 +8,12 @@ repository = "https://github.com/bevyengine/naga_oil/" readme = "README.md" [features] -default = ["test_shader"] +default = ["test_shader", "warn_deprecated"] # enable tests that need a graphical card test_shader = [] prune = [] override_any = [] +warn_deprecated = [] [dependencies] naga = { version = "0.13", features = ["wgsl-in", "wgsl-out", "glsl-in", "glsl-out", "clone", "span"] } diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index a9464b5..0288f7f 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -1,5 +1,7 @@ use std::collections::HashMap; +use tracing::warn; + use super::{ tokenizer::{Token, Tokenizer}, Composer, ImportDefWithOffset, ImportDefinition, @@ -44,6 +46,9 @@ pub fn parse_imports<'a>( // support deprecated #import mod item if let Some(Token::Identifier(..)) = tokens.peek() { + #[cfg(feature="warn_deprecated")] + warn!("item list imports are deprecated, please use `rust::style::item_imports;`\n| {}", input); + is_deprecated_itemlist = true; stack.push(format!("{}::", current)); current = String::default(); From 80f0a4bb8841d6a2193ef549a56985f1c3533fd4 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:37:30 +0100 Subject: [PATCH 25/30] fix duplicate test --- src/compose/parse_imports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 0288f7f..8e741d9 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -351,7 +351,7 @@ fn import_tokens() { assert!(test_parse(input).is_err()); let input = r" - #import a::b::{c}} + #import a::b::{{c} "; assert!(test_parse(input).is_err()); From feb2bd0cca828e1048dbaf725c16339313afc20d Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:48:28 +0100 Subject: [PATCH 26/30] add perf comment --- src/compose/preprocess.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 78fc7c3..15e3713 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -276,6 +276,9 @@ impl Preprocessor { open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); + // TODO: it's bad that we allocate here. ideally we would use something like + // let import_lines = &shader_str[initial_offset..offset] + // but we need the comments removed, and the iterator approach doesn't make that easy import_lines.push_str(&line); import_lines.push('\n'); @@ -410,6 +413,9 @@ impl Preprocessor { open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); + // TODO: it's bad that we allocate here. ideally we would use something like + // let import_lines = &shader_str[initial_offset..offset] + // but we need the comments removed, and the iterator approach doesn't make that easy import_lines.push_str(&line); import_lines.push('\n'); From 6daace43f4969903a16dcdf17f1638d46cb3a08d Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:49:18 +0100 Subject: [PATCH 27/30] fmt --- src/compose/mod.rs | 2 +- src/compose/parse_imports.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 5c5ef06..36a2070 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -883,7 +883,7 @@ impl Composer { }, } }; - + // ensure overrides are applied to virtual functions let raw_module_name = Self::decode(&target_module); let module_set = self.module_sets.get(&raw_module_name); diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 8e741d9..a8bae73 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -46,7 +46,7 @@ pub fn parse_imports<'a>( // support deprecated #import mod item if let Some(Token::Identifier(..)) = tokens.peek() { - #[cfg(feature="warn_deprecated")] + #[cfg(feature = "warn_deprecated")] warn!("item list imports are deprecated, please use `rust::style::item_imports;`\n| {}", input); is_deprecated_itemlist = true; From 856136af711a47ff19e6e4ad2d5f3e9c1de89c94 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Wed, 18 Oct 2023 12:55:22 +0100 Subject: [PATCH 28/30] change feature to "allow_deprecated" --- Cargo.toml | 4 ++-- src/compose/parse_imports.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9feb826..b2a34e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,12 +8,12 @@ repository = "https://github.com/bevyengine/naga_oil/" readme = "README.md" [features] -default = ["test_shader", "warn_deprecated"] +default = ["test_shader"] # enable tests that need a graphical card test_shader = [] prune = [] override_any = [] -warn_deprecated = [] +allow_deprecated = [] [dependencies] naga = { version = "0.13", features = ["wgsl-in", "wgsl-out", "glsl-in", "glsl-out", "clone", "span"] } diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index a8bae73..5442735 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -46,8 +46,8 @@ pub fn parse_imports<'a>( // support deprecated #import mod item if let Some(Token::Identifier(..)) = tokens.peek() { - #[cfg(feature = "warn_deprecated")] - warn!("item list imports are deprecated, please use `rust::style::item_imports;`\n| {}", input); + #[cfg(not(feature = "allow_deprecated"))] + warn!("item list imports are deprecated, please use `rust::style::item_imports; (or use feature `allow_deprecated`)`\n| {}", input); is_deprecated_itemlist = true; stack.push(format!("{}::", current)); From 5d7eaafc19f637efb55fe2467171df0b2872a63a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 18 Oct 2023 12:22:29 -0700 Subject: [PATCH 29/30] Fix unused import and swap ; for ` in warning message --- src/compose/parse_imports.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/compose/parse_imports.rs b/src/compose/parse_imports.rs index 5442735..50d9384 100644 --- a/src/compose/parse_imports.rs +++ b/src/compose/parse_imports.rs @@ -1,7 +1,5 @@ use std::collections::HashMap; -use tracing::warn; - use super::{ tokenizer::{Token, Tokenizer}, Composer, ImportDefWithOffset, ImportDefinition, @@ -47,7 +45,7 @@ pub fn parse_imports<'a>( // support deprecated #import mod item if let Some(Token::Identifier(..)) = tokens.peek() { #[cfg(not(feature = "allow_deprecated"))] - warn!("item list imports are deprecated, please use `rust::style::item_imports; (or use feature `allow_deprecated`)`\n| {}", input); + tracing::warn!("item list imports are deprecated, please use `rust::style::item_imports` (or use feature `allow_deprecated`)`\n| {}", input); is_deprecated_itemlist = true; stack.push(format!("{}::", current)); From 39dd0908940bdd1e3f8d0ab08e1abba5fa8e8fe5 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 18 Oct 2023 12:22:55 -0700 Subject: [PATCH 30/30] Add additional PERF comment and use PERF instead of TODO --- src/compose/preprocess.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index 15e3713..e805b05 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -273,10 +273,12 @@ impl Preprocessor { final_string.extend(std::iter::repeat(" ").take(line.len())); offset += line.len() + 1; + // PERF: Ideally we don't do multiple `match_indices` passes over `line` + // in addition to the final pass for the import parse open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); - // TODO: it's bad that we allocate here. ideally we would use something like + // PERF: it's bad that we allocate here. ideally we would use something like // let import_lines = &shader_str[initial_offset..offset] // but we need the comments removed, and the iterator approach doesn't make that easy import_lines.push_str(&line); @@ -410,10 +412,12 @@ impl Preprocessor { let initial_offset = offset; loop { + // PERF: Ideally we don't do multiple `match_indices` passes over `line` + // in addition to the final pass for the import parse open_count += line.match_indices('{').count(); open_count = open_count.saturating_sub(line.match_indices('}').count()); - // TODO: it's bad that we allocate here. ideally we would use something like + // PERF: it's bad that we allocate here. ideally we would use something like // let import_lines = &shader_str[initial_offset..offset] // but we need the comments removed, and the iterator approach doesn't make that easy import_lines.push_str(&line);