fix(traits)!: trait functions with a default implementation must not …

…be followed by a semicolon (noir-lang#2987)
Sakapoi · Oct 19, 2023 · 94d512e · 94d512e
1 parent 07dac1b
commit 94d512e
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 17 deletions.
diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs
@@ -394,8 +394,7 @@ fn trait_body() -> impl NoirParser<Vec<TraitItem>> {
     trait_function_declaration()
         .or(trait_type_declaration())
         .or(trait_constant_declaration())
-        .separated_by(just(Token::Semicolon))
-        .allow_trailing()
+        .repeated()
 }
 
 fn optional_default_value() -> impl NoirParser<Option<Expression>> {
@@ -408,18 +407,22 @@ fn trait_constant_declaration() -> impl NoirParser<TraitItem> {
         .then_ignore(just(Token::Colon))
         .then(parse_type())
         .then(optional_default_value())
+        .then_ignore(just(Token::Semicolon))
         .map(|((name, typ), default_value)| TraitItem::Constant { name, typ, default_value })
 }
 
 /// trait_function_declaration: 'fn' ident generics '(' declaration_parameters ')' function_return_type
 fn trait_function_declaration() -> impl NoirParser<TraitItem> {
+    let trait_function_body_or_semicolon =
+        block(expression()).map(Option::from).or(just(Token::Semicolon).map(|_| Option::None));
+
     keyword(Keyword::Fn)
         .ignore_then(ident())
         .then(generics())
         .then(parenthesized(function_declaration_parameters()))
         .then(function_return_type().map(|(_, typ)| typ))
         .then(where_clause())
-        .then(block(expression()).or_not())
+        .then(trait_function_body_or_semicolon)
         .validate(
             |(((((name, generics), parameters), return_type), where_clause), body), span, emit| {
                 validate_where_clause(&generics, &where_clause, span, emit);
@@ -526,7 +529,10 @@ fn function_declaration_parameters() -> impl NoirParser<Vec<(Ident, UnresolvedTy
 
 /// trait_type_declaration: 'type' ident generics
 fn trait_type_declaration() -> impl NoirParser<TraitItem> {
-    keyword(Keyword::Type).ignore_then(ident()).map(|name| TraitItem::Type { name })
+    keyword(Keyword::Type)
+        .ignore_then(ident())
+        .then_ignore(just(Token::Semicolon))
+        .map(|name| TraitItem::Type { name })
 }
 
 /// Parses a non-trait implementation, adding a set of methods to a type.
@@ -2042,7 +2048,7 @@ mod test {
                 // for a particular operation. Also known as `tag` or `marker` traits:
                 // https://stackoverflow.com/questions/71895489/what-is-the-purpose-of-defining-empty-impl-in-rust
                 "trait Empty {}",
-                "trait TraitWithDefaultBody { fn foo(self) {}; }",
+                "trait TraitWithDefaultBody { fn foo(self) {} }",
                 "trait TraitAcceptingMutableRef { fn foo(&mut self); }",
                 "trait TraitWithTypeBoundOperation { fn identity() -> Self; }",
                 "trait TraitWithAssociatedType { type Element; fn item(self, index: Field) -> Self::Element; }",

diff --git a/tooling/nargo_cli/tests/execution_success/trait_function_calls/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_function_calls/src/main.nr
@@ -30,7 +30,7 @@
 trait Trait1a {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 7892 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field {
         43278
     }
@@ -42,7 +42,7 @@ impl Trait1a for Struct1a { }
 trait Trait1b {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 2832 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field {
         9323
     }
@@ -58,7 +58,7 @@ impl Trait1b for Struct1b {
 trait Trait1c {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 7635 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field;
 }
 struct Struct1c { vl: Field }
@@ -72,7 +72,7 @@ impl Trait1c for Struct1c {
 trait Trait1d {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 2825 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field {
         29341
     }
@@ -88,7 +88,7 @@ impl Trait1d for Struct1d {
 trait Trait1e {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 85465 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field {
         2381
     }
@@ -107,7 +107,7 @@ impl Trait1e for Struct1e {
 trait Trait1f {
     fn trait_method1(self) -> Field {
         self.trait_method2() * 43257 - self.vl
-    };
+    }
     fn trait_method2(self) -> Field;
 }
 struct Struct1f { vl: Field }

diff --git a/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_override_implementation/src/main.nr
@@ -26,10 +26,10 @@ impl Default for Foo {
 
 trait F {
     fn f1(self) -> Field;
-    fn f2(self) -> Field { 2 };
-    fn f3(self) -> Field { 3 };
-    fn f4(self) -> Field { 4 };
-    fn f5(self) -> Field { 5 };
+    fn f2(self) -> Field { 2 }
+    fn f3(self) -> Field { 3 }
+    fn f4(self) -> Field { 4 }
+    fn f5(self) -> Field { 5 }
 }
 
 struct Bar {}

diff --git a/tooling/nargo_cli/tests/execution_success/trait_self/src/main.nr b/tooling/nargo_cli/tests/execution_success/trait_self/src/main.nr
@@ -3,11 +3,11 @@ trait ATrait {
 
     fn static_method() -> Field {
         Self::static_method_2()
-    };
+    }
 
     fn static_method_2() -> Field {
         100
-    };
+    }
 }
 
 struct Foo {