From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by sourceware.org (Postfix) with ESMTPS id 57AAA3857C74 for ; Tue, 30 Jan 2024 12:11:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 57AAA3857C74 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 57AAA3857C74 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::433 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706616735; cv=none; b=C730tOnKK9ycFGsOi6UopdCxYSjlHJr8YnMT+7ODipLjyWlYADzFU69Qx9/ooWimi6YLBazgzl2onCvS304tVZDGKlg5d9KSQDZqu7jhJUrDthWhkTzgEt/NyftbLmbABF98lXtDqa+H8H0iuwHTMysulwA5AuAXFemdvfxzMC4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706616735; c=relaxed/simple; bh=K3O0XWr4vho+6n/DB5PEktfoM4VgaNBAr3FnitrdPKg=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=TCG73eglZ5cWlRG80nPTPMc4y0LEaOiqYr06SMdYi2AmvdNFFn5rYDsGe2YMA8a7USJfF5JFQysgmIya7s5pL6QpoSMKllTSFAXmkbH0SjWcQ+OrOLFk0ZQoJRIuA+60V0P8CSBb0zyBbJL/gYMIzfK/BobY7+bnsrRa8VZX9dk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-x433.google.com with SMTP id ffacd0b85a97d-33ae7681d03so1637441f8f.1 for ; Tue, 30 Jan 2024 04:11:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; t=1706616696; x=1707221496; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=3EkNSh0m0U1J8MQDp6z2G26r1f5+x3wHw7ItH4T4/Nw=; b=K7UJmHo+9/FtpfDUfuMwtrKHpSYMJj9BXVZgIr0IRQVSpg59La4bLAbljz6HjRInFX zjkozK2VarX1vHIX8BgBVooXndqxrxuRnHkijv8NAuiXlRkE9eUv5hofiBo2BTBKesN+ PVifUkviK14ceFzwp/3H3z2EiJjWstNhu0g/KHnt/IkEOxywytYx0mNMLdcNP0kng/vw O15r0R8iu7IRBm3llYifUSTRS+1aNI0Rv49gCZoNuEn0+TR4fSWpSlRaZJ6BHCd24sRY enjdDCMUTw8T1tnDmidXHUuEb+PXjxIg4FAu8MyOHKM+MKZnt3cavAr9Wp9HFRXsIsPs PzPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706616696; x=1707221496; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=3EkNSh0m0U1J8MQDp6z2G26r1f5+x3wHw7ItH4T4/Nw=; b=Wc6nLTnpg3jpK/uMJe57fG6hTq6qeUKihUFEeeWSSmzjERI8buBYkZolUlJfN5wMSS Cz0tOmFBpECeY9Dm3ODnLDnwtUIe+cicu8rCeHn4ntMpp2pD79kGHZOL5s54V9ZzyO5Y ScfquNnXPtz6HQvGhlyY4KxgGvlB+/Q+IERWd3U9ajbpySwgZWc78x36nzLZ9GKalhzG MP/K+IsQeRmcMdkFIn/Xl6VoKyPq5ynhCrA3i20j1R6KWhgIXPKDdqvoOZXPYSegeEz4 lSjSQBO/Ba8yyacpStX11Nw+2xu+gSyeIrCNrdqvsIAxdi82UhRP7LGNOy2xxTyT5uHo Ob7A== X-Gm-Message-State: AOJu0YxD8OXkRjEE8Bze2z9Qh8MwI38PgsU9wYtV+f1EAq8WpVoT8Ibt 0t/cesNQer0yS5LphGidmQLhAwmMyizGMz7MQLIawI5YoBFyEdRPSbfPznxhuYoeJCnWk2GvZBb bQg== X-Google-Smtp-Source: AGHT+IFVurQ0sSqraQJgJzQz3JScksFdn6lEfRRhXZ622Gb9B+uWJPNp3T5wS2Dw9X1ywKnC3E69YA== X-Received: by 2002:a5d:6d82:0:b0:337:33a:c880 with SMTP id l2-20020a5d6d82000000b00337033ac880mr4465350wrs.26.1706616696138; Tue, 30 Jan 2024 04:11:36 -0800 (PST) Received: from platypus.localdomain ([62.23.166.218]) by smtp.gmail.com with ESMTPSA id f9-20020a056000036900b00339307d9d31sm10569894wrf.112.2024.01.30.04.11.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jan 2024 04:11:35 -0800 (PST) From: arthur.cohen@embecosm.com To: gcc-patches@gcc.gnu.org Cc: gcc-rust@gcc.gnu.org, Pierre-Emmanuel Patry Subject: [COMMITTED 062/101] gccrs: Make function bodies truly optional Date: Tue, 30 Jan 2024 13:07:18 +0100 Message-ID: <20240130121026.807464-65-arthur.cohen@embecosm.com> X-Mailer: git-send-email 2.42.1 In-Reply-To: <20240130121026.807464-2-arthur.cohen@embecosm.com> References: <20240130121026.807464-2-arthur.cohen@embecosm.com> Reply-To: arthur.cohen@embecosm.com MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: From: Pierre-Emmanuel Patry 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 --- 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 . */ #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> 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 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 ¶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 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 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 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 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 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::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 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