From 2107b205619dfa329214bfcbe1b6b5dd38445966 Mon Sep 17 00:00:00 2001 From: Igor Shaposhnik Date: Wed, 24 Nov 2021 16:28:23 +0300 Subject: [PATCH] [wgsl-in] Don't allow keywords to be used as identifiers --- src/back/wgsl/writer.rs | 8 ++- src/front/wgsl/mod.rs | 46 +++++++++++-- src/keywords/mod.rs | 4 +- src/lib.rs | 2 +- tests/in/access.wgsl | 2 +- tests/in/pointers.wgsl | 8 +-- tests/out/spv/pointers.spvasm | 2 +- tests/out/wgsl/access.wgsl | 2 +- tests/out/wgsl/pointers.wgsl | 8 +-- tests/wgsl-errors.rs | 126 +++++++++++++++++++++++++++++++--- 10 files changed, 176 insertions(+), 32 deletions(-) diff --git a/src/back/wgsl/writer.rs b/src/back/wgsl/writer.rs index 6d5b2016a3..9b04346d0e 100644 --- a/src/back/wgsl/writer.rs +++ b/src/back/wgsl/writer.rs @@ -72,8 +72,12 @@ impl Writer { fn reset(&mut self, module: &Module) { self.names.clear(); - self.namer - .reset(module, crate::keywords::wgsl::RESERVED, &[], &mut self.names); + self.namer.reset( + module, + crate::keywords::wgsl::RESERVED, + &[], + &mut self.names, + ); self.named_expressions.clear(); self.ep_results.clear(); } diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index 3c2dc29ef7..60584c0f99 100644 --- a/src/front/wgsl/mod.rs +++ b/src/front/wgsl/mod.rs @@ -169,6 +169,7 @@ pub enum Error<'a> { Pointer(&'static str, Span), NotPointer(Span), NotReference(&'static str, Span), + ReservedKeyword(Span), Other, } @@ -431,6 +432,11 @@ impl<'a> Error<'a> { labels: vec![(span.clone(), "expression is a pointer".into())], notes: vec![], }, + Error::ReservedKeyword(ref name_span) => ParseError { + message: format!("Name `{}` is a reserved keyword", &source[name_span.clone()]), + labels: vec![(name_span.clone(), format!("definition of `{}`", &source[name_span.clone()]).into())], + notes: vec![], + }, Error::Other => ParseError { message: "other error".to_string(), labels: vec![], @@ -2079,15 +2085,18 @@ impl Parser { // Only set span if it's a named constant. Otherwise, the enclosing Expression should have // the span. - let span = NagaSpan::from(self.pop_scope(lexer)); + let span = self.pop_scope(lexer); let handle = if let Some(name) = register_name { + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(span)); + } const_arena.append( crate::Constant { name: Some(name.to_string()), specialization: None, inner, }, - span, + NagaSpan::from(span), ) } else { const_arena.fetch_or_append( @@ -2701,15 +2710,17 @@ impl Parser { } let bind_span = self.pop_scope(lexer); - - let name = match lexer.next() { - (Token::Word(word), _) => word, + let (name, span) = match lexer.next() { + (Token::Word(word), span) => (word, span), (Token::Paren('}'), _) => { let span = Layouter::round_up(alignment, offset); return Ok((members, span)); } other => return Err(Error::Unexpected(other, ExpectedToken::FieldName)), }; + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(span)); + } lexer.expect(Token::Separator(':'))?; let (ty, _access) = self.parse_type_decl(lexer, None, type_arena, const_arena)?; lexer.expect(Token::Separator(';'))?; @@ -3245,6 +3256,9 @@ impl Parser { let _ = lexer.next(); emitter.start(context.expressions); let (name, name_span) = lexer.next_ident_with_span()?; + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(name_span)); + } let given_ty = if lexer.skip(Token::Separator(':')) { let (ty, _access) = self.parse_type_decl( lexer, @@ -3300,6 +3314,9 @@ impl Parser { } let (name, name_span) = lexer.next_ident_with_span()?; + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(name_span)); + } let given_ty = if lexer.skip(Token::Separator(':')) { let (ty, _access) = self.parse_type_decl( lexer, @@ -3810,7 +3827,10 @@ impl Parser { self.push_scope(Scope::FunctionDecl, lexer); // read function name let mut lookup_ident = FastHashMap::default(); - let fun_name = lexer.next_ident()?; + let (fun_name, span) = lexer.next_ident_with_span()?; + if crate::keywords::wgsl::RESERVED.contains(&fun_name) { + return Err(Error::ReservedKeyword(span)); + } // populate initial expressions let mut expressions = Arena::new(); for (&name, expression) in lookup_global_expression.iter() { @@ -3845,6 +3865,9 @@ impl Parser { let mut binding = self.parse_varying_binding(lexer)?; let (param_name, param_name_span, param_type, _access) = self.parse_variable_ident_decl(lexer, &mut module.types, &mut module.constants)?; + if crate::keywords::wgsl::RESERVED.contains(¶m_name) { + return Err(Error::ReservedKeyword(param_name_span)); + } let param_index = arguments.len() as u32; let expression = expressions.append( crate::Expression::FunctionArgument(param_index), @@ -4016,7 +4039,10 @@ impl Parser { match lexer.next() { (Token::Separator(';'), _) => {} (Token::Word("struct"), _) => { - let name = lexer.next_ident()?; + let (name, span) = lexer.next_ident_with_span()?; + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(span)); + } let (members, span) = self.parse_struct_body(lexer, &mut module.types, &mut module.constants)?; let type_span = NagaSpan::from(lexer.span_from(start)); @@ -4048,6 +4074,9 @@ impl Parser { } (Token::Word("let"), _) => { let (name, name_span) = lexer.next_ident_with_span()?; + if crate::keywords::wgsl::RESERVED.contains(&name) { + return Err(Error::ReservedKeyword(name_span)); + } let given_ty = if lexer.skip(Token::Separator(':')) { let (ty, _access) = self.parse_type_decl( lexer, @@ -4093,6 +4122,9 @@ impl Parser { (Token::Word("var"), _) => { let pvar = self.parse_variable_decl(lexer, &mut module.types, &mut module.constants)?; + if crate::keywords::wgsl::RESERVED.contains(&pvar.name) { + return Err(Error::ReservedKeyword(pvar.name_span)); + } let var_handle = module.global_variables.append( crate::GlobalVariable { name: Some(pvar.name.to_owned()), diff --git a/src/keywords/mod.rs b/src/keywords/mod.rs index 67101a1c16..4f959bcd08 100644 --- a/src/keywords/mod.rs +++ b/src/keywords/mod.rs @@ -1,2 +1,2 @@ -#[cfg(any(feature = "wgsl-in", feature = "wgsl-out"))] -pub mod wgsl; \ No newline at end of file +#[cfg(any(feature = "wgsl-in", feature = "wgsl-out"))] +pub mod wgsl; diff --git a/src/lib.rs b/src/lib.rs index d1a0e0ef22..eea9d2e932 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,10 +209,10 @@ mod arena; pub mod back; mod block; pub mod front; +pub mod keywords; pub mod proc; mod span; pub mod valid; -pub(crate) mod keywords; pub use crate::arena::{Arena, Handle, Range, UniqueArena}; diff --git a/tests/in/access.wgsl b/tests/in/access.wgsl index 4a2228fa3c..034d0d5ab2 100644 --- a/tests/in/access.wgsl +++ b/tests/in/access.wgsl @@ -30,7 +30,7 @@ fn foo([[builtin(vertex_index)]] vi: u32) -> [[builtin(position)]] vec4 { let a = bar.data[arrayLength(&bar.data) - 2u]; // test pointer types - let pointer: ptr = &bar.data[0]; + let data_pointer: ptr = &bar.data[0]; let foo_value = read_from_private(&foo); // test storage stores diff --git a/tests/in/pointers.wgsl b/tests/in/pointers.wgsl index d21080d30b..e3160774c3 100644 --- a/tests/in/pointers.wgsl +++ b/tests/in/pointers.wgsl @@ -6,7 +6,7 @@ fn f() { [[block]] struct DynamicArray { - array: array; + arr: array; }; [[group(0), binding(0)]] @@ -15,12 +15,12 @@ var dynamic_array: DynamicArray; fn index_unsized(i: i32, v: u32) { let p: ptr = &dynamic_array; - let val = (*p).array[i]; - (*p).array[i] = val + v; + let val = (*p).arr[i]; + (*p).arr[i] = val + v; } fn index_dynamic_array(i: i32, v: u32) { - let p: ptr, read_write> = &dynamic_array.array; + let p: ptr, read_write> = &dynamic_array.arr; let val = (*p)[i]; (*p)[i] = val + v; diff --git a/tests/out/spv/pointers.spvasm b/tests/out/spv/pointers.spvasm index 698829fd52..661a8e2c2e 100644 --- a/tests/out/spv/pointers.spvasm +++ b/tests/out/spv/pointers.spvasm @@ -8,7 +8,7 @@ OpExtension "SPV_KHR_storage_buffer_storage_class" %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpSource GLSL 450 -OpMemberName %8 0 "array" +OpMemberName %8 0 "arr" OpName %8 "DynamicArray" OpName %11 "dynamic_array" OpName %12 "v" diff --git a/tests/out/wgsl/access.wgsl b/tests/out/wgsl/access.wgsl index ad1666f9a6..21efceff0a 100644 --- a/tests/out/wgsl/access.wgsl +++ b/tests/out/wgsl/access.wgsl @@ -25,7 +25,7 @@ fn foo([[builtin(vertex_index)]] vi: u32) -> [[builtin(position)]] vec4 { let arr: array,2> = bar.arr; let b: f32 = bar.matrix[3][0]; let a: i32 = bar.data[(arrayLength((&bar.data)) - 2u)]; - let pointer_: ptr = (&bar.data[0]); + let data_pointer: ptr = (&bar.data[0]); let e25: f32 = read_from_private((&foo_1)); bar.matrix[1][2] = 1.0; bar.matrix = mat4x4(vec4(0.0), vec4(1.0), vec4(2.0), vec4(3.0)); diff --git a/tests/out/wgsl/pointers.wgsl b/tests/out/wgsl/pointers.wgsl index 5c3d0e1f8a..4bf53b8a84 100644 --- a/tests/out/wgsl/pointers.wgsl +++ b/tests/out/wgsl/pointers.wgsl @@ -1,6 +1,6 @@ [[block]] struct DynamicArray { - array_: [[stride(4)]] array; + arr: [[stride(4)]] array; }; [[group(0), binding(0)]] @@ -15,13 +15,13 @@ fn f() { } fn index_unsized(i: i32, v_1: u32) { - let val: u32 = dynamic_array.array_[i]; - dynamic_array.array_[i] = (val + v_1); + let val: u32 = dynamic_array.arr[i]; + dynamic_array.arr[i] = (val + v_1); return; } fn index_dynamic_array(i_1: i32, v_2: u32) { - let p: ptr, read_write> = (&dynamic_array.array_); + let p: ptr, read_write> = (&dynamic_array.arr); let val_1: u32 = (*p)[i_1]; (*p)[i_1] = (val_1 + v_2); return; diff --git a/tests/wgsl-errors.rs b/tests/wgsl-errors.rs index 0109245c0b..571a34962a 100644 --- a/tests/wgsl-errors.rs +++ b/tests/wgsl-errors.rs @@ -114,18 +114,18 @@ fn negative_index() { fn bad_texture() { check( r#" - [[group(0), binding(0)]] var sampler : sampler; + [[group(0), binding(0)]] var sampler1 : sampler; [[stage(fragment)]] fn main() -> [[location(0)]] vec4 { let a = 3; - return textureSample(a, sampler, vec2(0.0)); + return textureSample(a, sampler1, vec2(0.0)); } "#, r#"error: expected an image, but found 'a' which is not an image ┌─ wgsl:7:38 │ -7 │ return textureSample(a, sampler, vec2(0.0)); +7 │ return textureSample(a, sampler1, vec2(0.0)); │ ^ not an image "#, @@ -154,12 +154,12 @@ fn bad_type_cast() { fn bad_texture_sample_type() { check( r#" - [[group(0), binding(0)]] var sampler : sampler; + [[group(0), binding(0)]] var sampler1 : sampler; [[group(0), binding(1)]] var texture : texture_2d; [[stage(fragment)]] fn main() -> [[location(0)]] vec4 { - return textureSample(texture, sampler, vec2(0.0)); + return textureSample(texture, sampler1, vec2(0.0)); } "#, r#"error: texture sample type must be one of f32, i32 or u32, but found bool @@ -327,13 +327,13 @@ fn unknown_type() { fn unknown_storage_format() { check( r#" - let storage: texture_storage_1d; + let storage1: texture_storage_1d; "#, r#"error: unknown storage format: 'rgba' - ┌─ wgsl:2:45 + ┌─ wgsl:2:46 │ -2 │ let storage: texture_storage_1d; - │ ^^^^ unknown storage format +2 │ let storage1: texture_storage_1d; + │ ^^^^ unknown storage format "#, ); @@ -531,6 +531,114 @@ fn postfix_pointers() { ); } +#[test] +fn reserved_keyword() { + // global var + check( + r#" + var bool: bool = true; + "#, + r###"error: Name ` bool: bool = true;` is a reserved keyword + ┌─ wgsl:2:16 + │ +2 │ var bool: bool = true; + │ ^^^^^^^^^^^^^^^^^^^ definition of ` bool: bool = true;` + +"###, + ); + + // global constant + check( + r#" + let break: bool = true; + fn foo() { + var foo = break; + } + "#, + r###"error: Name `break` is a reserved keyword + ┌─ wgsl:2:17 + │ +2 │ let break: bool = true; + │ ^^^^^ definition of `break` + +"###, + ); + + // local let + check( + r#" + fn foo() { + let atomic: f32 = 1.0; + } + "#, + r###"error: Name `atomic` is a reserved keyword + ┌─ wgsl:3:21 + │ +3 │ let atomic: f32 = 1.0; + │ ^^^^^^ definition of `atomic` + +"###, + ); + + // local var + check( + r#" + fn foo() { + var sampler: f32 = 1.0; + } + "#, + r###"error: Name `sampler` is a reserved keyword + ┌─ wgsl:3:21 + │ +3 │ var sampler: f32 = 1.0; + │ ^^^^^^^ definition of `sampler` + +"###, + ); + + // fn name + check( + r#" + fn break() {} + "#, + r###"error: Name `break` is a reserved keyword + ┌─ wgsl:2:16 + │ +2 │ fn break() {} + │ ^^^^^ definition of `break` + +"###, + ); + + // struct + check( + r#" + struct array {}; + "#, + r###"error: Name `array` is a reserved keyword + ┌─ wgsl:2:20 + │ +2 │ struct array {}; + │ ^^^^^ definition of `array` + +"###, + ); + + // struct member + check( + r#" + struct Foo { sampler: f32; }; + "#, + r###"error: Name `sampler` is a reserved keyword + ┌─ wgsl:2:26 + │ +2 │ struct Foo { sampler: f32; }; + │ ^^^^^^^ definition of `sampler` + +"###, + ); +} + macro_rules! check_validation_error { // We want to support an optional guard expression after the pattern, so // that we can check values we can't match against, like strings.