ast/function: Remove [set|add]_decorations()

Move them to the constructor

Bug: tint:390
Change-Id: I30bb6a1de060b790bf5202194d020d4e3889a307
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35008
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/reader/spirv/function.cc

@@ -694,14 +694,13 @@ bool FunctionEmitter::EmitFunctionDeclaration() {
   if (failed()) {
     return false;
   }
-
-  auto* ast_fn = create<ast::Function>(name, std::move(ast_params), ret_ty,
-                                       create<ast::BlockStatement>());
-
+  ast::FunctionDecorationList decos;
   if (ep_info_ != nullptr) {
-    ast_fn->add_decoration(
-        create<ast::StageDecoration>(ep_info_->stage, Source{}));
+    decos.emplace_back(create<ast::StageDecoration>(ep_info_->stage, Source{}));
   }
+  auto* ast_fn =
+      create<ast::Function>(Source{}, name, std::move(ast_params), ret_ty,
+                            create<ast::BlockStatement>(), std::move(decos));
 
   ast_module_.AddFunction(ast_fn);
 

src/reader/wgsl/parser_impl.cc

@@ -186,6 +186,21 @@ struct BlockCounters {
 
 }  // namespace
 
+ParserImpl::FunctionHeader::FunctionHeader() = default;
+
+ParserImpl::FunctionHeader::FunctionHeader(const FunctionHeader&) = default;
+
+ParserImpl::FunctionHeader::FunctionHeader(Source src,
+                                           std::string n,
+                                           ast::VariableList p,
+                                           ast::type::Type* ret_ty)
+    : source(src), name(n), params(p), return_type(ret_ty) {}
+
+ParserImpl::FunctionHeader::~FunctionHeader() = default;
+
+ParserImpl::FunctionHeader& ParserImpl::FunctionHeader::operator=(
+    const FunctionHeader& rhs) = default;
+
 ParserImpl::ParserImpl(Source::File const* file)
     : lexer_(std::make_unique<Lexer>(file)) {}
 
@@ -1238,8 +1253,8 @@ Expect<ast::StructMember*> ParserImpl::expect_struct_member(
 // function_decl
 //   : function_header body_stmt
 Maybe<ast::Function*> ParserImpl::function_decl(ast::DecorationList& decos) {
-  auto f = function_header();
-  if (f.errored) {
+  auto header = function_header();
+  if (header.errored) {
     if (sync_to(Token::Type::kBraceLeft, /* consume: */ false)) {
       // There were errors in the function header, but the parser has managed to
       // resynchronize with the opening brace. As there's no outer
@@ -1251,7 +1266,7 @@ Maybe<ast::Function*> ParserImpl::function_decl(ast::DecorationList& decos) {
     }
     return Failure::kErrored;
   }
-  if (!f.matched)
+  if (!header.matched)
     return Failure::kNoMatch;
 
   bool errored = false;
@@ -1260,8 +1275,6 @@ Maybe<ast::Function*> ParserImpl::function_decl(ast::DecorationList& decos) {
   if (func_decos.errored)
     errored = true;
 
-  f->set_decorations(std::move(func_decos.value));
-
   auto body = expect_body_stmt();
   if (body.errored)
     errored = true;
@@ -1269,8 +1282,9 @@ Maybe<ast::Function*> ParserImpl::function_decl(ast::DecorationList& decos) {
   if (errored)
     return Failure::kErrored;
 
-  f->set_body(body.value);
-  return f.value;
+  return create<ast::Function>(header->source, header->name, header->params,
+                               header->return_type, body.value,
+                               func_decos.value);
 }
 
 // function_type_decl
@@ -1285,7 +1299,7 @@ Maybe<ast::type::Type*> ParserImpl::function_type_decl() {
 
 // function_header
 //   : FN IDENT PAREN_LEFT param_list PAREN_RIGHT ARROW function_type_decl
-Maybe<ast::Function*> ParserImpl::function_header() {
+Maybe<ParserImpl::FunctionHeader> ParserImpl::function_header() {
   Source source;
   if (!match(Token::Type::kFn, &source))
     return Failure::kNoMatch;
@@ -1320,8 +1334,8 @@ Maybe<ast::Function*> ParserImpl::function_header() {
   if (errored)
     return Failure::kErrored;
 
-  return create<ast::Function>(source, name.value, std::move(params.value),
-                               type.value, create<ast::BlockStatement>());
+  return FunctionHeader{source, name.value, std::move(params.value),
+                        type.value};
 }
 
 // param_list

src/reader/wgsl/parser_impl.h

@@ -229,6 +229,39 @@ class ParserImpl {
     Source source;
   };
 
+  /// FunctionHeader contains the parsed information for a function header.
+  struct FunctionHeader {
+    /// Constructor
+    FunctionHeader();
+    /// Copy constructor
+    /// @param other the FunctionHeader to copy
+    FunctionHeader(const FunctionHeader& other);
+    /// Constructor
+    /// @param src parsed header source
+    /// @param n function name
+    /// @param p function parameters
+    /// @param ret_ty function return type
+    FunctionHeader(Source src,
+                   std::string n,
+                   ast::VariableList p,
+                   ast::type::Type* ret_ty);
+    /// Destructor
+    ~FunctionHeader();
+    /// Assignment operator
+    /// @param other the FunctionHeader to copy
+    /// @returns this FunctionHeader
+    FunctionHeader& operator=(const FunctionHeader& other);
+
+    /// Parsed header source
+    Source source;
+    /// Function name
+    std::string name;
+    /// Function parameters
+    ast::VariableList params;
+    /// Function return type
+    ast::type::Type* return_type;
+  };
+
   /// Creates a new parser using the given file
   /// @param file the input source file to parse
   explicit ParserImpl(Source::File const* file);
@@ -398,8 +431,8 @@ class ParserImpl {
   /// @returns the parsed type or nullptr otherwise
   Maybe<ast::type::Type*> function_type_decl();
   /// Parses a `function_header` grammar element
-  /// @returns the parsed function nullptr otherwise
-  Maybe<ast::Function*> function_header();
+  /// @returns the parsed function header
+  Maybe<FunctionHeader> function_header();
   /// Parses a `param_list` grammar element, erroring on parse failure.
   /// @returns the parsed variables
   Expect<ast::VariableList> expect_param_list();

src/reader/wgsl/parser_impl_function_header_test.cc

@@ -30,13 +30,12 @@ TEST_F(ParserImplTest, FunctionHeader) {
   ASSERT_FALSE(p->has_error()) << p->error();
   EXPECT_TRUE(f.matched);
   EXPECT_FALSE(f.errored);
-  ASSERT_NE(f.value, nullptr);
 
-  EXPECT_EQ(f->name(), "main");
-  ASSERT_EQ(f->params().size(), 2u);
-  EXPECT_EQ(f->params()[0]->name(), "a");
-  EXPECT_EQ(f->params()[1]->name(), "b");
-  EXPECT_TRUE(f->return_type()->Is<ast::type::Void>());
+  EXPECT_EQ(f->name, "main");
+  ASSERT_EQ(f->params.size(), 2u);
+  EXPECT_EQ(f->params[0]->name(), "a");
+  EXPECT_EQ(f->params[1]->name(), "b");
+  EXPECT_TRUE(f->return_type->Is<ast::type::Void>());
 }
 
 TEST_F(ParserImplTest, FunctionHeader_MissingIdent) {
@@ -45,7 +44,6 @@ TEST_F(ParserImplTest, FunctionHeader_MissingIdent) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:4: expected identifier for function declaration");
 }
 
@@ -55,7 +53,6 @@ TEST_F(ParserImplTest, FunctionHeader_InvalidIdent) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:4: expected identifier for function declaration");
 }
 
@@ -65,7 +62,6 @@ TEST_F(ParserImplTest, FunctionHeader_MissingParenLeft) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:8: expected '(' for function declaration");
 }
 
@@ -75,7 +71,6 @@ TEST_F(ParserImplTest, FunctionHeader_InvalidParamList) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:16: expected identifier for parameter");
 }
 
@@ -85,7 +80,6 @@ TEST_F(ParserImplTest, FunctionHeader_MissingParenRight) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:10: expected ')' for function declaration");
 }
 
@@ -95,7 +89,6 @@ TEST_F(ParserImplTest, FunctionHeader_MissingArrow) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:11: expected '->' for function declaration");
 }
 
@@ -105,7 +98,6 @@ TEST_F(ParserImplTest, FunctionHeader_InvalidReturnType) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:14: unknown constructed type 'invalid'");
 }
 
@@ -115,7 +107,6 @@ TEST_F(ParserImplTest, FunctionHeader_MissingReturnType) {
   EXPECT_FALSE(f.matched);
   EXPECT_TRUE(f.errored);
   EXPECT_TRUE(p->has_error());
-  EXPECT_EQ(f.value, nullptr);
   EXPECT_EQ(p->error(), "1:13: unable to determine function return type");
 }