From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7905) id 3C46B3858295; Tue, 30 Jan 2024 11:59:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3C46B3858295 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1706615972; bh=bWxlYi/TIiAH6NYifn2yszQ8DODaE3I4JHWw6QfJqDE=; h=From:To:Subject:Date:From; b=slYAK+3vlmZlC8Nv3ewJcTkeiKmHyb/4+iYPwUzccCfqoUrboABnt0EIKEYaZ+rLb QX3gZ/pmbhNpMvkoNtwnWOllnrTYRmNIM5Z11DoxYwJHZWy+oayWI4zmaerNnp9OO/ GMZruXWJ9tI3TGMG9bxYyqtY1FYZJskSv4V30OYA= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Arthur Cohen To: gcc-cvs@gcc.gnu.org Subject: [gcc r14-8586] gccrs: Make function bodies truly optional X-Act-Checkin: gcc X-Git-Author: Pierre-Emmanuel Patry X-Git-Refname: refs/heads/trunk X-Git-Oldrev: f5e04e40490e57bff2da0568ab9e67a33e644898 X-Git-Newrev: 7a989394795a740dd4bcabbba251428cc9df08f1 Message-Id: <20240130115932.3C46B3858295@sourceware.org> Date: Tue, 30 Jan 2024 11:59:32 +0000 (GMT) List-Id: https://gcc.gnu.org/g:7a989394795a740dd4bcabbba251428cc9df08f1 commit r14-8586-g7a989394795a740dd4bcabbba251428cc9df08f1 Author: Pierre-Emmanuel Patry Date: Wed Nov 22 15:15:29 2023 +0100 gccrs: 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 Diff: --- 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 647724bec11f..d5a98f1ccc78 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 4ec5c7cf1d0a..230a152b05bd 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 607f07955d43..b9096032d41b 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 . */ #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 9a83f3d59818..a995273de123 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> function_params; std::unique_ptr return_type; WhereClause where_clause; - std::unique_ptr function_body; + tl::optional> 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> generic_params, std::vector> function_params, std::unique_ptr return_type, WhereClause where_clause, - std::unique_ptr function_body, Visibility vis, - std::vector outer_attrs, location_t locus, - bool is_default = false) + tl::optional> function_body, + Visibility vis, std::vector 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 &get_definition () + tl::optional> &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 fd1156546185..089520182ac9 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 1745af061746..ad473c2dcb0e 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 ¶m : 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 096a30e1e423..6f904dde19f0 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 function_body = std::unique_ptr ( - 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 8d6ab7cd3506..2895872f336c 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 function_body = std::unique_ptr ( - 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 acceec302a2d..52766afd9c42 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::parse_function (AST::Visibility vis, // parse where clause - if exists AST::WhereClause where_clause = parse_where_clause (); - // parse block expression - std::unique_ptr block_expr = parse_block_expr (); + tl::optional> body = tl::nullopt; + if (lexer.peek_token ()->get_id () == SEMICOLON) + lexer.skip_token (); + else + { + std::unique_ptr block_expr = parse_block_expr (); + if (block_expr != nullptr) + body = std::move (block_expr); + } return std::unique_ptr ( 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::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> 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 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 ( - 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 ( - 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 ( + 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::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> body = tl::nullopt; - lexer.skip_token (); - return nullptr; - } - std::unique_ptr 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 ( diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc index eaee5bc86061..60eca5b13c7b 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 293c98fd6f29..f9aa93ba7c49 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 1ab174b6caa4..c1ed3cea1136 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 a1e23082e880..3c296b565f52 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