Make function bodies truly optional

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>

diff --git a/gcc/rust/ast/rust-ast-collector.cc b/gcc/rust/ast/rust-ast-collector.cc
index 647724bec11fd2ece7dabce058a50ccb7e65722b..d5a98f1ccc7817b91148a59b7fac33866d61ae69 100644 (file)
--- 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 4ec5c7cf1d0a77beed2bc3cf4fc925d6741f1119..230a152b05bd18dc13d98f8034842b0baa3193d0 100644 (file)
--- 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 15d7880bce45c93261a95c96b4a7d6a0b481d611..88ac8608bb62ec39b10468228b380fa5180bb162 100644 (file)
--- 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 bed8312497ac32f88cf310fc0fea3f1202507991..685f9493370ed0aaec2a9abe03cacba3689bbdd0 100644 (file)
--- 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 7e50db6b1c578553919961567b20f23fcfc4b411..36fe3a1512645c91b0d297d105b7e39403ae8352 100644 (file)
--- 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 1745af061746a32a91858875e09a020a11f432ae..ad473c2dcb0e52b5fe832b213f142ad221369890 100644 (file)
--- 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 91602c9efbb14055f361a001c0329e2a82d68cfe..249f9d88a6bbc5ceb2e2204b645e513afc3e4d3f 100644 (file)
--- 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 c024acf60f98c9e80b8c56f1803e7e415cfd7fa8..01494ae028c8002875b4902390eb7847b2a341ce 100644 (file)
--- 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 b8c40c3c1577a3b91348f7fd49d78d1c1adec3a2..12c34d98956fb3c97c88e2aa3be82b667c233aad 100644 (file)
--- 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 31565043522d67f23c117382fe03a229ee1aebc3..fc08098b433eeea29f30823c1016af1123bb9b4d 100644 (file)
--- 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 3be5f128a15d920fb98e845347e41011c9390e61..c1111a6159a2799fe50485a007cb0e76aa963e71 100644 (file)
--- 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 1ab174b6caa43a4c424a41c8d6a1d98e11242566..c1ed3cea1136ce1f3fd455386e24e8806536de42 100644 (file)
--- 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 5a43224f01c7a90ea10544a5a7b7494d7b7e20be..4975f10890f517b711c7dba0c011717ad02bfb36 100644 (file)
--- 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