commit cb907b5bc22129b38c6b992a93c54484c5e65b1b
author dan sinclair <dsinclair@chromium.org> Mon Sep 21 15:55:32 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Sep 21 15:55:32 2020 +0000
tree 30be0a5a7be2ae44e7b771b4fb465cccafcdcb68
parent 43d17780ca5ad4becf99e1d17ed71083ba97c80c

[wgsl-reader] Fix issue with missing open brace for functions.

This Cl fixes the function parsing so the opening brace is required.
This will return an error now. The statement::body_stmt is changed to
make the open paren optional.

Bug: tint:236
Change-Id: I3faee1fd5add19aebcdb86943d6f4190c03d0f7a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/28520
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>

src/reader/wgsl/parser_impl.cc

diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index c3f79f3..27ffa72 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -2089,19 +2089,21 @@
 //   : BRACKET_LEFT statements BRACKET_RIGHT
 std::unique_ptr<ast::BlockStatement> ParserImpl::body_stmt() {
   auto t = peek();
-  if (!t.IsBraceLeft())
-    return {};
+  if (!t.IsBraceLeft()) {
+    set_error(t, "missing {");
+    return nullptr;
+  }
 
   next();  // Consume the peek
 
   auto stmts = statements();
   if (has_error())
-    return {};
+    return nullptr;
 
   t = next();
   if (!t.IsBraceRight()) {
     set_error(t, "missing }");
-    return {};
+    return nullptr;
   }
 
   return stmts;
@@ -2165,7 +2167,7 @@
 //   | continue_stmt SEMICOLON
 //   | DISCARD SEMICOLON
 //   | assignment_stmt SEMICOLON
-//   | body_stmt
+//   | body_stmt?
 std::unique_ptr<ast::Statement> ParserImpl::statement() {
   auto t = peek();
   if (t.IsSemicolon()) {
@@ -2281,11 +2283,14 @@
     return assign;
   }
 
-  auto body = body_stmt();
-  if (has_error())
-    return nullptr;
-  if (body != nullptr)
-    return body;
+  t = peek();
+  if (t.IsBraceLeft()) {
+    auto body = body_stmt();
+    if (has_error())
+      return nullptr;
+    if (body != nullptr)
+      return body;
+  }
 
   return nullptr;
 }
@@ -2392,12 +2397,6 @@
     return nullptr;
   }
 
-  t = peek();
-  if (!t.IsBraceLeft()) {
-    set_error(t, "missing {");
-    return nullptr;
-  }
-
   auto body = body_stmt();
   if (has_error())
     return nullptr;
@@ -2440,12 +2439,6 @@
       return {};
     }
 
-    t = peek();
-    if (!t.IsBraceLeft()) {
-      set_error(t, "missing {");
-      return {};
-    }
-
     auto body = body_stmt();
     if (has_error())
       return {};
@@ -2471,12 +2464,6 @@
   auto source = t.source();
   next();  // Consume the peek
 
-  t = peek();
-  if (!t.IsBraceLeft()) {
-    set_error(t, "missing {");
-    return nullptr;
-  }
-
   auto body = body_stmt();
   if (has_error())
     return nullptr;
@@ -2889,8 +2876,8 @@
   if (!t.IsContinuing()) {
     return std::make_unique<ast::BlockStatement>();
   }
-
   next();  // Consume the peek
+
   return body_stmt();
 }
 

src/reader/wgsl/parser_impl_function_decl_test.cc

diff --git a/src/reader/wgsl/parser_impl_function_decl_test.cc b/src/reader/wgsl/parser_impl_function_decl_test.cc
index 3ae5905..00558a7 100644
--- a/src/reader/wgsl/parser_impl_function_decl_test.cc
+++ b/src/reader/wgsl/parser_impl_function_decl_test.cc
@@ -169,6 +169,14 @@
   EXPECT_EQ(p->error(), "1:28: missing ;");
 }
 
+TEST_F(ParserImplTest, FunctionDecl_MissingLeftBrace) {
+  auto* p = parser("fn main() -> void return; }");
+  auto f = p->function_decl();
+  ASSERT_TRUE(p->has_error());
+  ASSERT_EQ(f, nullptr);
+  EXPECT_EQ(p->error(), "1:19: missing {");
+}
+
 }  // namespace
 }  // namespace wgsl
 }  // namespace reader