[wgsl-in] Error on param redefinition (#2342)

* error on param redefinition * also fix #2312
gfx-rs · May 25, 2023 · 1c17fa8 · 1c17fa8
1 parent baed251
commit 1c17fa8
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 50 deletions.
diff --git a/src/front/wgsl/parse/mod.rs b/src/front/wgsl/parse/mod.rs
@@ -84,6 +84,18 @@ impl<'a> ExpressionContext<'a, '_, '_> {
         }
         Ok(accumulator)
     }
+
+    fn declare_local(&mut self, name: ast::Ident<'a>) -> Result<Handle<ast::Local>, Error<'a>> {
+        let handle = self.locals.append(ast::Local, name.span);
+        if let Some(old) = self.local_table.add(name.name, handle) {
+            Err(Error::Redefinition {
+                previous: self.locals.get_span(old),
+                current: name.span,
+            })
+        } else {
+            Ok(handle)
+        }
+    }
 }
 
 /// Which grammar rule we are in the midst of parsing.
@@ -1612,14 +1624,7 @@ impl Parser {
                         let expr_id = self.general_expression(lexer, ctx.reborrow())?;
                         lexer.expect(Token::Separator(';'))?;
 
-                        let handle = ctx.locals.append(ast::Local, name.span);
-                        if let Some(old) = ctx.local_table.add(name.name, handle) {
-                            return Err(Error::Redefinition {
-                                previous: ctx.locals.get_span(old),
-                                current: name.span,
-                            });
-                        }
-
+                        let handle = ctx.declare_local(name)?;
                         ast::StatementKind::LocalDecl(ast::LocalDecl::Let(ast::Let {
                             name,
                             ty: given_ty,
@@ -1647,14 +1652,7 @@ impl Parser {
 
                         lexer.expect(Token::Separator(';'))?;
 
-                        let handle = ctx.locals.append(ast::Local, name.span);
-                        if let Some(old) = ctx.local_table.add(name.name, handle) {
-                            return Err(Error::Redefinition {
-                                previous: ctx.locals.get_span(old),
-                                current: name.span,
-                            });
-                        }
-
+                        let handle = ctx.declare_local(name)?;
                         ast::StatementKind::LocalDecl(ast::LocalDecl::Var(ast::LocalVariable {
                             name,
                             ty,
@@ -2013,15 +2011,15 @@ impl Parser {
         ctx.local_table.push_scope();
 
         lexer.expect(Token::Paren('{'))?;
-        let mut statements = ast::Block::default();
+        let mut block = ast::Block::default();
         while !lexer.skip(Token::Paren('}')) {
-            self.statement(lexer, ctx.reborrow(), &mut statements)?;
+            self.statement(lexer, ctx.reborrow(), &mut block)?;
         }
 
         ctx.local_table.pop_scope();
 
         let span = self.pop_rule_span(lexer);
-        Ok((statements, span))
+        Ok((block, span))
     }
 
     fn varying_binding<'a>(
@@ -2060,6 +2058,9 @@ impl Parser {
             unresolved: dependencies,
         };
 
+        // start a scope that contains arguments as well as the function body
+        ctx.local_table.push_scope();
+
         // read parameter list
         let mut arguments = Vec::new();
         lexer.expect(Token::Paren('('))?;
@@ -2078,8 +2079,7 @@ impl Parser {
             lexer.expect(Token::Separator(':'))?;
             let param_type = self.type_decl(lexer, ctx.reborrow())?;
 
-            let handle = ctx.locals.append(ast::Local, param_name.span);
-            ctx.local_table.add(param_name.name, handle);
+            let handle = ctx.declare_local(param_name)?;
             arguments.push(ast::FunctionArgument {
                 name: param_name,
                 ty: param_type,
@@ -2097,8 +2097,14 @@ impl Parser {
             None
         };
 
-        // read body
-        let body = self.block(lexer, ctx)?.0;
+        // do not use `self.block` here, since we must not push a new scope
+        lexer.expect(Token::Paren('{'))?;
+        let mut body = ast::Block::default();
+        while !lexer.skip(Token::Paren('}')) {
+            self.statement(lexer, ctx.reborrow(), &mut body)?;
+        }
+
+        ctx.local_table.pop_scope();
 
         let fun = ast::Function {
             entry_point: None,

diff --git a/tests/in/lexical-scopes.wgsl b/tests/in/lexical-scopes.wgsl
@@ -1,45 +1,41 @@
 fn blockLexicalScope(a: bool) {
-    let a = 1.0;
     {
         let a = 2;
         {
-            let a = true;
+            let a = 2.0;
         }
-        let test = a == 3;
+        let test: i32 = a;
     }
-    let test = a == 2.0;
+    let test: bool = a;
 }
 
 fn ifLexicalScope(a: bool) {
-    let a = 1.0;
-    if (a == 1.0) {
-        let a = true;
+    if (a) {
+        let a = 2.0;
     }
-    let test = a == 2.0;
+    let test: bool = a;
 }
 
 
 fn loopLexicalScope(a: bool) {
-    let a = 1.0;
     loop {
-        let a = true;
+        let a = 2.0;
     }
-    let test = a == 2.0;
+    let test: bool = a;
 }
 
 fn forLexicalScope(a: f32) {
-    let a = false;
     for (var a = 0; a < 1; a++) {
-        let a = 3.0;
+        let a = true;
     }
-    let test = a == true;
+    let test: f32 = a;
 }
 
 fn whileLexicalScope(a: i32) {
     while (a > 2) {
         let a = false;
     }
-    let test = a == 1;
+    let test: i32 = a;
 }
 
 fn switchLexicalScope(a: i32) {

diff --git a/tests/out/wgsl/lexical-scopes.wgsl b/tests/out/wgsl/lexical-scopes.wgsl
@@ -1,42 +1,43 @@
 fn blockLexicalScope(a: bool) {
     {
         {
+            return;
         }
-        let test = (2 == 3);
     }
-    let test_1 = (1.0 == 2.0);
 }
 
 fn ifLexicalScope(a_1: bool) {
-    if (1.0 == 1.0) {
+    if a_1 {
+        return;
+    } else {
+        return;
     }
-    let test_2 = (1.0 == 2.0);
 }
 
 fn loopLexicalScope(a_2: bool) {
     loop {
     }
-    let test_3 = (1.0 == 2.0);
+    return;
 }
 
 fn forLexicalScope(a_3: f32) {
     var a_4: i32;
 
     a_4 = 0;
     loop {
-        let _e4 = a_4;
-        if (_e4 < 1) {
+        let _e3 = a_4;
+        if (_e3 < 1) {
         } else {
             break;
         }
         {
         }
         continuing {
-            let _e8 = a_4;
-            a_4 = (_e8 + 1);
+            let _e7 = a_4;
+            a_4 = (_e7 + 1);
         }
     }
-    let test_4 = (false == true);
+    return;
 }
 
 fn whileLexicalScope(a_5: i32) {
@@ -48,7 +49,7 @@ fn whileLexicalScope(a_5: i32) {
         {
         }
     }
-    let test_5 = (a_5 == 1);
+    return;
 }
 
 fn switchLexicalScope(a_6: i32) {
@@ -60,6 +61,6 @@ fn switchLexicalScope(a_6: i32) {
         default: {
         }
     }
-    let test_6 = (a_6 == 2);
+    let test = (a_6 == 2);
 }
 
diff --git a/tests/wgsl-errors.rs b/tests/wgsl-errors.rs
@@ -1883,6 +1883,44 @@ fn function_returns_void() {
     )
 }
 
+#[test]
+fn function_param_redefinition_as_param() {
+    check(
+        "
+        fn x(a: f32, a: vec2<f32>) {}
+    ",
+        r###"error: redefinition of `a`
+  ┌─ wgsl:2:14
+  │
+2 │         fn x(a: f32, a: vec2<f32>) {}
+  │              ^       ^ redefinition of `a`
+  │              │        
+  │              previous definition of `a`
+
+"###,
+    )
+}
+
+#[test]
+fn function_param_redefinition_as_local() {
+    check(
+        "
+        fn x(a: f32) {
+			let a = 0.0;
+		}
+    ",
+        r###"error: redefinition of `a`
+  ┌─ wgsl:2:14
+  │
+2 │         fn x(a: f32) {
+  │              ^ previous definition of `a`
+3 │             let a = 0.0;
+  │                 ^ redefinition of `a`
+
+"###,
+    )
+}
+
 #[test]
 fn binding_array_local() {
     check_validation! {