[COMMITTED 062/101] gccrs: Make function bodies truly optional

[arthur.cohen@embecosm.com]

From: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>

Missing body on a function should be rejected at a later stage in the
compiler, not during parsing.

gcc/rust/ChangeLog:

	* ast/rust-ast-collector.cc (TokenCollector::visit): Adapt defintion
	getter.
	* ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Likewise.
	* expand/rust-cfg-strip.cc (CfgStrip::visit): Likewise.
	* expand/rust-expand-visitor.cc (ExpandVisitor::visit): Likewise.
	* hir/rust-ast-lower-implitem.h: Likewise.
	* hir/rust-ast-lower-item.cc (ASTLoweringItem::visit): Likewise.
	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Likewise.
	* resolve/rust-ast-resolve-stmt.h: Likewise.
	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Likewise.
	* util/rust-attributes.cc (AttributeChecker::visit):  Likewise.
	* parse/rust-parse-impl.h: Allow empty function body during parsing.
	* ast/rust-ast.cc (Function::Function): Constructor now take an
	optional for the body.
	(Function::operator=): Adapt to new optional member.
	(Function::as_string): Likewise.
	* ast/rust-item.h (class Function): Make body optional and do not
	rely on nullptr anymore.

Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.patry@embecosm.com>
---
 gcc/rust/ast/rust-ast-collector.cc        |   7 +-
 gcc/rust/ast/rust-ast-visitor.cc          |   3 +-
 gcc/rust/ast/rust-ast.cc                  |  24 +++--
 gcc/rust/ast/rust-item.h                  |  13 +--
 gcc/rust/expand/rust-cfg-strip.cc         |  16 ++--
 gcc/rust/expand/rust-expand-visitor.cc    |   8 +-
 gcc/rust/hir/rust-ast-lower-implitem.h    |   2 +-
 gcc/rust/hir/rust-ast-lower-item.cc       |   2 +-
 gcc/rust/parse/rust-parse-impl.h          | 105 ++++++++++------------
 gcc/rust/resolve/rust-ast-resolve-item.cc |   2 +-
 gcc/rust/resolve/rust-ast-resolve-stmt.h  |   2 +-
 gcc/rust/resolve/rust-default-resolver.cc |   3 +-
 gcc/rust/util/rust-attributes.cc          |   3 +-
 13 files changed, 92 insertions(+), 98 deletions(-)

diff --git a/gcc/rust/ast/rust-ast-collector.cc b/gcc/rust/ast/rust-ast-collector.cc
index 647724bec11..d5a98f1ccc7 100644
--- a/gcc/rust/ast/rust-ast-collector.cc
+++ b/gcc/rust/ast/rust-ast-collector.cc
@@ -1730,11 +1730,10 @@ TokenCollector::visit (Function &function)
   if (function.has_where_clause ())
     visit (function.get_where_clause ());
 
-  auto &block = function.get_definition ();
-  if (!block)
-    push (Rust::Token::make (SEMICOLON, UNDEF_LOCATION));
+  if (function.has_body ())
+    visit (*function.get_definition ());
   else
-    visit (block);
+    push (Rust::Token::make (SEMICOLON, UNDEF_LOCATION));
   newline ();
 }
 
diff --git a/gcc/rust/ast/rust-ast-visitor.cc b/gcc/rust/ast/rust-ast-visitor.cc
index 4ec5c7cf1d0..230a152b05b 100644
--- a/gcc/rust/ast/rust-ast-visitor.cc
+++ b/gcc/rust/ast/rust-ast-visitor.cc
@@ -777,7 +777,8 @@ DefaultASTVisitor::visit (AST::Function &function)
   if (function.has_return_type ())
     visit (function.get_return_type ());
   visit (function.get_where_clause ());
-  visit (function.get_definition ());
+  if (function.has_body ())
+    visit (*function.get_definition ());
 }
 
 void
diff --git a/gcc/rust/ast/rust-ast.cc b/gcc/rust/ast/rust-ast.cc
index 607f07955d4..b9096032d41 100644
--- a/gcc/rust/ast/rust-ast.cc
+++ b/gcc/rust/ast/rust-ast.cc
@@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
 #include "rust-ast.h"
+#include "optional.h"
 #include "rust-system.h"
 #include "rust-ast-full.h"
 #include "rust-diagnostics.h"
@@ -1100,8 +1101,10 @@ Function::Function (Function const &other)
     return_type = other.return_type->clone_type ();
 
   // guard to prevent null dereference (only required if error state)
-  if (other.function_body != nullptr)
-    function_body = other.function_body->clone_block_expr ();
+  if (other.has_body ())
+    function_body = other.function_body.value ()->clone_block_expr ();
+  else
+    function_body = tl::nullopt;
 
   generic_params.reserve (other.generic_params.size ());
   for (const auto &e : other.generic_params)
@@ -1131,10 +1134,10 @@ Function::operator= (Function const &other)
     return_type = nullptr;
 
   // guard to prevent null dereference (only required if error state)
-  if (other.function_body != nullptr)
-    function_body = other.function_body->clone_block_expr ();
+  if (other.has_body ())
+    function_body = other.function_body.value ()->clone_block_expr ();
   else
-    function_body = nullptr;
+    function_body = tl::nullopt;
 
   generic_params.reserve (other.generic_params.size ());
   for (const auto &e : other.generic_params)
@@ -1221,15 +1224,8 @@ Function::as_string () const
 
   str += "\n";
 
-  // DEBUG: null pointer check
-  if (function_body == nullptr)
-    {
-      rust_debug (
-	"something really terrible has gone wrong - null pointer function "
-	"body in function.");
-      return "NULL_POINTER_MARK";
-    }
-  str += function_body->as_string () + "\n";
+  if (has_body ())
+    str += function_body.value ()->as_string () + "\n";
 
   return str;
 }
diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
index 9a83f3d5981..a995273de12 100644
--- a/gcc/rust/ast/rust-item.h
+++ b/gcc/rust/ast/rust-item.h
@@ -1299,7 +1299,7 @@ class Function : public VisItem,
   std::vector<std::unique_ptr<Param>> function_params;
   std::unique_ptr<Type> return_type;
   WhereClause where_clause;
-  std::unique_ptr<BlockExpr> function_body;
+  tl::optional<std::unique_ptr<BlockExpr>> function_body;
   location_t locus;
   bool is_default;
 
@@ -1323,14 +1323,16 @@ public:
     return function_params.size () > 0 && function_params[0]->is_self ();
   }
 
+  bool has_body () const { return function_body.has_value (); }
+
   // Mega-constructor with all possible fields
   Function (Identifier function_name, FunctionQualifiers qualifiers,
 	    std::vector<std::unique_ptr<GenericParam>> generic_params,
 	    std::vector<std::unique_ptr<Param>> function_params,
 	    std::unique_ptr<Type> return_type, WhereClause where_clause,
-	    std::unique_ptr<BlockExpr> function_body, Visibility vis,
-	    std::vector<Attribute> outer_attrs, location_t locus,
-	    bool is_default = false)
+	    tl::optional<std::unique_ptr<BlockExpr>> function_body,
+	    Visibility vis, std::vector<Attribute> outer_attrs,
+	    location_t locus, bool is_default = false)
     : VisItem (std::move (vis), std::move (outer_attrs)),
       qualifiers (std::move (qualifiers)),
       function_name (std::move (function_name)),
@@ -1390,9 +1392,8 @@ public:
   }
 
   // TODO: is this better? Or is a "vis_block" better?
-  std::unique_ptr<BlockExpr> &get_definition ()
+  tl::optional<std::unique_ptr<BlockExpr>> &get_definition ()
   {
-    rust_assert (function_body != nullptr);
     return function_body;
   }
 
diff --git a/gcc/rust/expand/rust-cfg-strip.cc b/gcc/rust/expand/rust-cfg-strip.cc
index fd115654618..089520182ac 100644
--- a/gcc/rust/expand/rust-cfg-strip.cc
+++ b/gcc/rust/expand/rust-cfg-strip.cc
@@ -2031,13 +2031,17 @@ CfgStrip::visit (AST::Function &function)
   /* body should always exist - if error state, should have returned
    * before now */
   // can't strip block itself, but can strip sub-expressions
-  auto &block_expr = function.get_definition ();
-  block_expr->accept_vis (*this);
-  if (block_expr->is_marked_for_strip ())
-    rust_error_at (block_expr->get_locus (),
-		   "cannot strip block expression in this position - outer "
-		   "attributes not allowed");
+  if (function.has_body ())
+    {
+      auto &block_expr = function.get_definition ().value ();
+      block_expr->accept_vis (*this);
+      if (block_expr->is_marked_for_strip ())
+	rust_error_at (block_expr->get_locus (),
+		       "cannot strip block expression in this position - outer "
+		       "attributes not allowed");
+    }
 }
+
 void
 CfgStrip::visit (AST::TypeAlias &type_alias)
 {
diff --git a/gcc/rust/expand/rust-expand-visitor.cc b/gcc/rust/expand/rust-expand-visitor.cc
index 1745af06174..ad473c2dcb0 100644
--- a/gcc/rust/expand/rust-expand-visitor.cc
+++ b/gcc/rust/expand/rust-expand-visitor.cc
@@ -1001,8 +1001,9 @@ ExpandVisitor::visit (AST::UseDeclaration &use_decl)
 void
 ExpandVisitor::visit (AST::Function &function)
 {
-  visit_inner_using_attrs (function,
-			   function.get_definition ()->get_inner_attrs ());
+  if (function.has_body ())
+    visit_inner_using_attrs (
+      function, function.get_definition ().value ()->get_inner_attrs ());
   for (auto &param : function.get_generic_params ())
     visit (param);
 
@@ -1014,7 +1015,8 @@ ExpandVisitor::visit (AST::Function &function)
   if (function.has_where_clause ())
     expand_where_clause (function.get_where_clause ());
 
-  visit (function.get_definition ());
+  if (function.has_body ())
+    visit (*function.get_definition ());
 }
 
 void
diff --git a/gcc/rust/hir/rust-ast-lower-implitem.h b/gcc/rust/hir/rust-ast-lower-implitem.h
index 096a30e1e42..6f904dde19f 100644
--- a/gcc/rust/hir/rust-ast-lower-implitem.h
+++ b/gcc/rust/hir/rust-ast-lower-implitem.h
@@ -168,7 +168,7 @@ public:
     bool terminated = false;
     std::unique_ptr<HIR::BlockExpr> function_body
       = std::unique_ptr<HIR::BlockExpr> (
-	ASTLoweringBlock::translate (function.get_definition ().get (),
+	ASTLoweringBlock::translate (function.get_definition ()->get (),
 				     &terminated));
 
     auto crate_num = mappings->get_current_crate ();
diff --git a/gcc/rust/hir/rust-ast-lower-item.cc b/gcc/rust/hir/rust-ast-lower-item.cc
index 8d6ab7cd350..2895872f336 100644
--- a/gcc/rust/hir/rust-ast-lower-item.cc
+++ b/gcc/rust/hir/rust-ast-lower-item.cc
@@ -446,7 +446,7 @@ ASTLoweringItem::visit (AST::Function &function)
   bool terminated = false;
   std::unique_ptr<HIR::BlockExpr> function_body
     = std::unique_ptr<HIR::BlockExpr> (
-      ASTLoweringBlock::translate (function.get_definition ().get (),
+      ASTLoweringBlock::translate (function.get_definition ()->get (),
 				   &terminated));
 
   auto crate_num = mappings->get_current_crate ();
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index acceec302a2..52766afd9c4 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -23,6 +23,7 @@
  * This is also the reason why there are no include guards. */
 
 #include "rust-common.h"
+#include "rust-expr.h"
 #include "rust-item.h"
 #include "rust-common.h"
 #include "rust-token.h"
@@ -2976,14 +2977,21 @@ Parser<ManagedTokenSource>::parse_function (AST::Visibility vis,
   // parse where clause - if exists
   AST::WhereClause where_clause = parse_where_clause ();
 
-  // parse block expression
-  std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
+  tl::optional<std::unique_ptr<AST::BlockExpr>> body = tl::nullopt;
+  if (lexer.peek_token ()->get_id () == SEMICOLON)
+    lexer.skip_token ();
+  else
+    {
+      std::unique_ptr<AST::BlockExpr> block_expr = parse_block_expr ();
+      if (block_expr != nullptr)
+	body = std::move (block_expr);
+    }
 
   return std::unique_ptr<AST::Function> (
     new AST::Function (std::move (function_name), std::move (qualifiers),
 		       std::move (generic_params), std::move (function_params),
 		       std::move (return_type), std::move (where_clause),
-		       std::move (block_expr), std::move (vis),
+		       std::move (body), std::move (vis),
 		       std::move (outer_attrs), locus));
 }
 
@@ -5710,49 +5718,33 @@ Parser<ManagedTokenSource>::parse_inherent_impl_function_or_method (
   // parse where clause (optional)
   AST::WhereClause where_clause = parse_where_clause ();
 
-  // parse function definition (in block) - semicolon not allowed
+  tl::optional<std::unique_ptr<AST::BlockExpr>> body = tl::nullopt;
   if (lexer.peek_token ()->get_id () == SEMICOLON)
+    lexer.skip_token ();
+  else
     {
-      Error error (lexer.peek_token ()->get_locus (),
-		   "%s declaration in inherent impl not allowed - must have "
-		   "a definition",
-		   is_method ? "method" : "function");
-      add_error (std::move (error));
+      auto result = parse_block_expr ();
 
-      lexer.skip_token ();
-      return nullptr;
-    }
-  std::unique_ptr<AST::BlockExpr> body = parse_block_expr ();
-  if (body == nullptr)
-    {
-      Error error (lexer.peek_token ()->get_locus (),
-		   "could not parse definition in inherent impl %s definition",
-		   is_method ? "method" : "function");
-      add_error (std::move (error));
+      if (result == nullptr)
+	{
+	  Error error (
+	    lexer.peek_token ()->get_locus (),
+	    "could not parse definition in inherent impl %s definition",
+	    is_method ? "method" : "function");
+	  add_error (std::move (error));
 
-      skip_after_end_block ();
-      return nullptr;
+	  skip_after_end_block ();
+	  return nullptr;
+	}
+      body = std::move (result);
     }
 
-  // do actual if instead of ternary for return value optimisation
-  if (is_method)
-    {
-      return std::unique_ptr<AST::Function> (
-	new AST::Function (std::move (ident), std::move (qualifiers),
-			   std::move (generic_params),
-			   std::move (function_params), std::move (return_type),
-			   std::move (where_clause), std::move (body),
-			   std::move (vis), std::move (outer_attrs), locus));
-    }
-  else
-    {
-      return std::unique_ptr<AST::Function> (
-	new AST::Function (std::move (ident), std::move (qualifiers),
-			   std::move (generic_params),
-			   std::move (function_params), std::move (return_type),
-			   std::move (where_clause), std::move (body),
-			   std::move (vis), std::move (outer_attrs), locus));
-    }
+  return std::unique_ptr<AST::Function> (
+    new AST::Function (std::move (ident), std::move (qualifiers),
+		       std::move (generic_params), std::move (function_params),
+		       std::move (return_type), std::move (where_clause),
+		       std::move (body), std::move (vis),
+		       std::move (outer_attrs), locus));
 }
 
 // Parses a single trait impl item (item inside a trait impl block).
@@ -5960,27 +5952,24 @@ Parser<ManagedTokenSource>::parse_trait_impl_function_or_method (
     "successfully parsed where clause in function or method trait impl item");
 
   // parse function definition (in block) - semicolon not allowed
-  if (lexer.peek_token ()->get_id () == SEMICOLON)
-    {
-      Error error (
-	lexer.peek_token ()->get_locus (),
-	"%s declaration in trait impl not allowed - must have a definition",
-	is_method ? "method" : "function");
-      add_error (std::move (error));
+  tl::optional<std::unique_ptr<AST::BlockExpr>> body = tl::nullopt;
 
-      lexer.skip_token ();
-      return nullptr;
-    }
-  std::unique_ptr<AST::BlockExpr> body = parse_block_expr ();
-  if (body == nullptr)
+  if (lexer.peek_token ()->get_id () == SEMICOLON)
+    lexer.skip_token ();
+  else
     {
-      Error error (lexer.peek_token ()->get_locus (),
-		   "could not parse definition in trait impl %s definition",
-		   is_method ? "method" : "function");
-      add_error (std::move (error));
+      auto result = parse_block_expr ();
+      if (result == nullptr)
+	{
+	  Error error (lexer.peek_token ()->get_locus (),
+		       "could not parse definition in trait impl %s definition",
+		       is_method ? "method" : "function");
+	  add_error (std::move (error));
 
-      skip_after_end_block ();
-      return nullptr;
+	  skip_after_end_block ();
+	  return nullptr;
+	}
+      body = std::move (result);
     }
 
   return std::unique_ptr<AST::Function> (
diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc
index eaee5bc8606..60eca5b13c7 100644
--- a/gcc/rust/resolve/rust-ast-resolve-item.cc
+++ b/gcc/rust/resolve/rust-ast-resolve-item.cc
@@ -616,7 +616,7 @@ ResolveItem::visit (AST::Function &function)
     }
 
   // resolve the function body
-  ResolveExpr::go (function.get_definition ().get (), path, cpath);
+  ResolveExpr::go (function.get_definition ()->get (), path, cpath);
 
   resolver->get_name_scope ().pop ();
   resolver->get_type_scope ().pop ();
diff --git a/gcc/rust/resolve/rust-ast-resolve-stmt.h b/gcc/rust/resolve/rust-ast-resolve-stmt.h
index 293c98fd6f2..f9aa93ba7c4 100644
--- a/gcc/rust/resolve/rust-ast-resolve-stmt.h
+++ b/gcc/rust/resolve/rust-ast-resolve-stmt.h
@@ -378,7 +378,7 @@ public:
       }
 
     // resolve the function body
-    ResolveExpr::go (function.get_definition ().get (), path, cpath);
+    ResolveExpr::go (function.get_definition ()->get (), path, cpath);
 
     resolver->get_name_scope ().pop ();
     resolver->get_type_scope ().pop ();
diff --git a/gcc/rust/resolve/rust-default-resolver.cc b/gcc/rust/resolve/rust-default-resolver.cc
index 1ab174b6caa..c1ed3cea113 100644
--- a/gcc/rust/resolve/rust-default-resolver.cc
+++ b/gcc/rust/resolve/rust-default-resolver.cc
@@ -77,7 +77,8 @@ DefaultResolver::visit (AST::Function &function)
 	  }
       }
 
-    function.get_definition ()->accept_vis (*this);
+    if (function.has_body ())
+      function.get_definition ().value ()->accept_vis (*this);
   };
 
   ctx.scoped (Rib::Kind::Function, function.get_node_id (), def_fn);
diff --git a/gcc/rust/util/rust-attributes.cc b/gcc/rust/util/rust-attributes.cc
index a1e23082e88..3c296b565f5 100644
--- a/gcc/rust/util/rust-attributes.cc
+++ b/gcc/rust/util/rust-attributes.cc
@@ -667,7 +667,8 @@ AttributeChecker::visit (AST::Function &fun)
       else if (result.name == "no_mangle")
 	check_no_mangle_function (attribute, fun);
     }
-  fun.get_definition ()->accept_vis (*this);
+  if (fun.has_body ())
+    fun.get_definition ().value ()->accept_vis (*this);
 }
 
 void
-- 
2.42.1