From: Pierre-Emmanuel Patry Date: Thu, 19 Oct 2023 13:23:26 +0000 (+0200) Subject: gccrs: Fix multiple issues with variadic representation X-Git-Tag: basepoints/gcc-15~2010 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2272cfb53e0721c4b7985d037d5e098621bee31a;p=thirdparty%2Fgcc.git gccrs: Fix multiple issues with variadic representation The new variadic representation has introduced multiple issues and ICE into the codebase. Some early passes in the compiler depend on the parameters all having a type and being an actual parameter. gcc/rust/ChangeLog: * ast/rust-ast.cc (ExternalFunctionItem::as_string): Adapt as_string function to the new ast representation. (NamedFunctionParam::as_string): Likewise. * ast/rust-item.h: Add a function to test whether a FunctionParam has a name pattern. * expand/rust-cfg-strip.cc (CfgStrip::visit): Adapt cfg strip visitor for the new variadic arguments. * hir/rust-ast-lower-extern.h: Adapt lowering to the new variadic function representation. * metadata/rust-export-metadata.cc (ExportContext::emit_function): Change call to constructor. * parse/rust-parse-impl.h (Parser::parse_named_function_param): Change NamedFunctionParam parsing to accomodate new variadic representation. (Parser::parse_external_item): Change external item parsing to use the new NamedFunctionParam variadics. * parse/rust-parse.h: Add new parsing function prototypes. * ast/rust-ast-collector.cc (TokenCollector::visit): Rework token collection to take into account variadic parameters. * ast/rust-ast-visitor.cc: Likewise. * expand/rust-expand-visitor.cc (ExpandVisitor::visit): Change function bound to avoid getting the type of a variadic parameter. * resolve/rust-ast-resolve-item.cc (ResolveExternItem::visit): Likewise. * resolve/rust-early-name-resolver.cc (EarlyNameResolver::visit): Likewise. Signed-off-by: Pierre-Emmanuel Patry --- diff --git a/gcc/rust/ast/rust-ast-collector.cc b/gcc/rust/ast/rust-ast-collector.cc index 71cc09835c3a..999752327216 100644 --- a/gcc/rust/ast/rust-ast-collector.cc +++ b/gcc/rust/ast/rust-ast-collector.cc @@ -172,9 +172,22 @@ TokenCollector::visit (Visitable &v) void TokenCollector::visit (FunctionParam ¶m) { - visit (param.get_pattern ()); - push (Rust::Token::make (COLON, UNDEF_LOCATION)); - visit (param.get_type ()); + visit_items_as_lines (param.get_outer_attrs ()); + if (!param.is_variadic ()) + { + visit (param.get_pattern ()); + push (Rust::Token::make (COLON, UNDEF_LOCATION)); + visit (param.get_type ()); + } + else + { + if (param.has_name ()) + { + visit (param.get_pattern ()); + push (Rust::Token::make (COLON, UNDEF_LOCATION)); + } + push (Rust::Token::make (ELLIPSIS, UNDEF_LOCATION)); + } } void @@ -293,9 +306,23 @@ void TokenCollector::visit (NamedFunctionParam ¶m) { auto name = param.get_name (); - push (Rust::Token::make_identifier (param.get_locus (), std::move (name))); - push (Rust::Token::make (COLON, UNDEF_LOCATION)); - visit (param.get_type ()); + if (!param.is_variadic ()) + { + push ( + Rust::Token::make_identifier (param.get_locus (), std::move (name))); + push (Rust::Token::make (COLON, UNDEF_LOCATION)); + visit (param.get_type ()); + } + else + { + if (name != "") + { + push (Rust::Token::make_identifier (param.get_locus (), + std::move (name))); + push (Rust::Token::make (COLON, UNDEF_LOCATION)); + } + push (Rust::Token::make (ELLIPSIS, UNDEF_LOCATION)); + } } void @@ -2209,13 +2236,6 @@ TokenCollector::visit (ExternalFunctionItem &function) push (Rust::Token::make (LEFT_PAREN, UNDEF_LOCATION)); visit_items_joined_by_separator (function.get_function_params ()); - if (function.is_variadic ()) - { - push (Rust::Token::make (COMMA, UNDEF_LOCATION)); - // TODO: Add variadic outer attributes? - // TODO: Add variadic name once implemented. - push (Rust::Token::make (ELLIPSIS, UNDEF_LOCATION)); - } push (Rust::Token::make (RIGHT_PAREN, UNDEF_LOCATION)); if (function.has_return_type ()) diff --git a/gcc/rust/ast/rust-ast-visitor.cc b/gcc/rust/ast/rust-ast-visitor.cc index 9fb10997e1fd..4b954fdc9984 100644 --- a/gcc/rust/ast/rust-ast-visitor.cc +++ b/gcc/rust/ast/rust-ast-visitor.cc @@ -721,8 +721,11 @@ void DefaultASTVisitor::visit (AST::FunctionParam ¶m) { visit_outer_attrs (param); - visit (param.get_pattern ()); - visit (param.get_type ()); + if (param.has_name ()) + visit (param.get_pattern ()); + + if (!param.is_variadic ()) + visit (param.get_type ()); } void @@ -1054,7 +1057,8 @@ void DefaultASTVisitor::visit (AST::NamedFunctionParam ¶m) { visit_outer_attrs (param); - visit (param.get_type ()); + if (!param.is_variadic ()) + visit (param.get_type ()); } void diff --git a/gcc/rust/ast/rust-ast.cc b/gcc/rust/ast/rust-ast.cc index d9c493debe6e..3e8d4143e5af 100644 --- a/gcc/rust/ast/rust-ast.cc +++ b/gcc/rust/ast/rust-ast.cc @@ -3036,7 +3036,7 @@ ExternalFunctionItem::as_string () const // function params str += "\n Function params: "; - if (function_params.empty () && !has_variadics) + if (function_params.empty ()) { str += "none"; } @@ -3044,21 +3044,6 @@ ExternalFunctionItem::as_string () const { for (const auto ¶m : function_params) str += "\n " + param.as_string (); - - if (has_variadics) - { - str += "\n variadic outer attrs: "; - if (has_variadic_outer_attrs ()) - { - for (const auto &attr : variadic_outer_attrs) - str += "\n " + attr.as_string (); - } - else - { - str += "none"; - } - str += "\n ... (variadic)"; - } } // add type on new line @@ -3080,9 +3065,13 @@ NamedFunctionParam::as_string () const { std::string str = append_attributes (outer_attrs, OUTER); - str += "\n" + name; + if (has_name ()) + str += "\n" + name; - str += "\n Type: " + param_type->as_string (); + if (is_variadic ()) + str += "..."; + else + str += "\n Type: " + param_type->as_string (); return str; } diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h index 7cdbc8baf85f..70ef93dcc529 100644 --- a/gcc/rust/ast/rust-item.h +++ b/gcc/rust/ast/rust-item.h @@ -618,6 +618,8 @@ public: return param_name; } + bool has_name () const { return param_name != nullptr; } + // TODO: is this better? Or is a "vis_block" better? std::unique_ptr &get_type () { @@ -4171,7 +4173,7 @@ class NamedFunctionParam public: /* Returns whether the named function parameter has a name (i.e. name is not * '_'). */ - bool has_name () const { return name != "_"; } + bool has_name () const { return name != "_" && name != ""; } bool has_outer_attrs () const { return !outer_attrs.empty (); } @@ -4182,6 +4184,8 @@ public: return param_type == nullptr && !variadic; } + bool is_variadic () const { return variadic; } + std::string get_name () const { return name; } location_t get_locus () { return locus; } @@ -4294,8 +4298,6 @@ class ExternalFunctionItem : public ExternalItem WhereClause where_clause; std::vector function_params; - bool has_variadics; - std::vector variadic_outer_attrs; public: // Returns whether item has generic parameters. @@ -4314,12 +4316,10 @@ public: bool has_visibility () const { return !visibility.is_error (); } // Returns whether item has variadic parameters. - bool is_variadic () const { return has_variadics; } - - // Returns whether item has outer attributes on its variadic parameters. - bool has_variadic_outer_attrs () const + bool is_variadic () const { - return !variadic_outer_attrs.empty (); + return function_params.size () != 0 + && function_params.back ().is_variadic (); } location_t get_locus () const { return locus; } @@ -4331,17 +4331,14 @@ public: Identifier item_name, std::vector> generic_params, std::unique_ptr return_type, WhereClause where_clause, - std::vector function_params, bool has_variadics, - std::vector variadic_outer_attrs, Visibility vis, + std::vector function_params, Visibility vis, std::vector outer_attrs, location_t locus) : ExternalItem (), outer_attrs (std::move (outer_attrs)), visibility (std::move (vis)), item_name (std::move (item_name)), locus (locus), generic_params (std::move (generic_params)), return_type (std::move (return_type)), where_clause (std::move (where_clause)), - function_params (std::move (function_params)), - has_variadics (has_variadics), - variadic_outer_attrs (std::move (variadic_outer_attrs)) + function_params (std::move (function_params)) { // TODO: assert that if has variadic outer attrs, then has_variadics is // true? @@ -4351,10 +4348,7 @@ public: ExternalFunctionItem (ExternalFunctionItem const &other) : outer_attrs (other.outer_attrs), visibility (other.visibility), item_name (other.item_name), locus (other.locus), - where_clause (other.where_clause), - function_params (other.function_params), - has_variadics (other.has_variadics), - variadic_outer_attrs (other.variadic_outer_attrs) + where_clause (other.where_clause), function_params (other.function_params) { node_id = other.node_id; // guard to prevent null pointer dereference @@ -4375,8 +4369,6 @@ public: locus = other.locus; where_clause = other.where_clause; function_params = other.function_params; - has_variadics = other.has_variadics; - variadic_outer_attrs = other.variadic_outer_attrs; node_id = other.node_id; // guard to prevent null pointer dereference diff --git a/gcc/rust/expand/rust-cfg-strip.cc b/gcc/rust/expand/rust-cfg-strip.cc index 9c5b92c4460c..8976d858e163 100644 --- a/gcc/rust/expand/rust-cfg-strip.cc +++ b/gcc/rust/expand/rust-cfg-strip.cc @@ -2603,12 +2603,15 @@ CfgStrip::visit (AST::ExternalFunctionItem &item) continue; } - auto &type = param.get_type (); - type->accept_vis (*this); + if (!param.is_variadic ()) + { + auto &type = param.get_type (); + param.get_type ()->accept_vis (*this); - if (type->is_marked_for_strip ()) - rust_error_at (type->get_locus (), - "cannot strip type in this position"); + if (type->is_marked_for_strip ()) + rust_error_at (type->get_locus (), + "cannot strip type in this position"); + } // increment if nothing else happens ++it; diff --git a/gcc/rust/expand/rust-expand-visitor.cc b/gcc/rust/expand/rust-expand-visitor.cc index b0fa004fdc34..cb9e8b6568ab 100644 --- a/gcc/rust/expand/rust-expand-visitor.cc +++ b/gcc/rust/expand/rust-expand-visitor.cc @@ -1265,12 +1265,9 @@ ExpandVisitor::visit (AST::ExternalFunctionItem &item) for (auto ¶m : item.get_generic_params ()) visit (param); - // FIXME: Should this work? What is the difference between NamedFunctionParam - // and FunctionParam? - // expand_function_params (item.get_function_params ()); - for (auto ¶m : item.get_function_params ()) - maybe_expand_type (param.get_type ()); + if (!param.is_variadic ()) + maybe_expand_type (param.get_type ()); if (item.has_return_type ()) maybe_expand_type (item.get_return_type ()); diff --git a/gcc/rust/hir/rust-ast-lower-extern.h b/gcc/rust/hir/rust-ast-lower-extern.h index 422e902253ad..cee480bb9ed8 100644 --- a/gcc/rust/hir/rust-ast-lower-extern.h +++ b/gcc/rust/hir/rust-ast-lower-extern.h @@ -110,7 +110,7 @@ public: translated = new HIR::ExternalFunctionItem ( mapping, function.get_identifier (), std::move (generic_params), std::unique_ptr (return_type), std::move (where_clause), - std::move (function_params), function.is_variadic (), std::move (vis), + std::move (function_params), is_variadic, std::move (vis), function.get_outer_attrs (), function.get_locus ()); } diff --git a/gcc/rust/metadata/rust-export-metadata.cc b/gcc/rust/metadata/rust-export-metadata.cc index 74c57ea68e1c..73c3abfe0a28 100644 --- a/gcc/rust/metadata/rust-export-metadata.cc +++ b/gcc/rust/metadata/rust-export-metadata.cc @@ -118,11 +118,12 @@ ExportContext::emit_function (const HIR::Function &fn) function_params.push_back (std::move (p)); } - AST::ExternalItem *external_item = new AST::ExternalFunctionItem ( - item_name, {} /* generic_params */, std::move (return_type), - where_clause, std::move (function_params), false /* has_variadics */, - {} /* variadic_outer_attrs */, vis, function.get_outer_attrs (), - function.get_locus ()); + AST::ExternalItem *external_item + = new AST::ExternalFunctionItem (item_name, {} /* generic_params */, + std::move (return_type), where_clause, + std::move (function_params), vis, + function.get_outer_attrs (), + function.get_locus ()); std::vector> external_items; external_items.push_back ( diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h index 61c915db9972..007b2bfaba04 100644 --- a/gcc/rust/parse/rust-parse-impl.h +++ b/gcc/rust/parse/rust-parse-impl.h @@ -5963,6 +5963,161 @@ Parser::parse_extern_block (AST::Visibility vis, std::move (outer_attrs), locus)); } +template +AST::NamedFunctionParam +Parser::parse_named_function_param () +{ + AST::AttrVec outer_attrs = parse_outer_attributes (); + location_t locus = lexer.peek_token ()->get_locus (); + + if (lexer.peek_token ()->get_id () == ELLIPSIS) + { + lexer.skip_token (); // Skip ellipsis + return AST::NamedFunctionParam (std::move (outer_attrs), locus); + } + + // parse identifier/_ + std::string name; + + const_TokenPtr t = lexer.peek_token (); + location_t name_location = t->get_locus (); + switch (t->get_id ()) + { + case IDENTIFIER: + name = t->get_str (); + lexer.skip_token (); + break; + case UNDERSCORE: + name = "_"; + lexer.skip_token (); + break; + default: + // this is not a function param, but not necessarily an error + return AST::NamedFunctionParam::create_error (); + } + + if (!skip_token (COLON)) + { + // skip after somewhere? + return AST::NamedFunctionParam::create_error (); + } + + // parse (required) type + std::unique_ptr param_type = parse_type (); + if (param_type == nullptr) + { + Error error ( + lexer.peek_token ()->get_locus (), + "could not parse param type in extern block function declaration"); + add_error (std::move (error)); + + skip_after_semicolon (); + return AST::NamedFunctionParam::create_error (); + } + + return AST::NamedFunctionParam (std::move (name), std::move (param_type), + std::move (outer_attrs), name_location); +} + +template +template +std::vector +Parser::parse_named_function_params ( + EndTokenPred is_end_token) +{ + std::vector params; + if (is_end_token (lexer.peek_token ()->get_id ())) + return params; + + auto initial_param = parse_named_function_param (); + if (initial_param.is_error ()) + return params; + + params.push_back (std::move (initial_param)); + auto t = lexer.peek_token (); + while (t->get_id () == COMMA) + { + lexer.skip_token (); + if (is_end_token (lexer.peek_token ()->get_id ())) + break; + + auto param = parse_named_function_param (); + if (param.is_error ()) + { + Error error (lexer.peek_token ()->get_locus (), + "failed to parse param in c function params"); + add_error (error); + return std::vector (); + } + params.push_back (std::move (param)); + t = lexer.peek_token (); + } + params.shrink_to_fit (); + return params; +} + +template +std::unique_ptr +Parser::parse_external_function_item ( + AST::Visibility vis, AST::AttrVec outer_attrs) +{ + location_t locus = lexer.peek_token ()->get_locus (); + + // parse extern function declaration item + // skip function token + lexer.skip_token (); + + // parse identifier + const_TokenPtr ident_tok = expect_token (IDENTIFIER); + if (ident_tok == nullptr) + { + skip_after_semicolon (); + return nullptr; + } + Identifier ident{ident_tok}; + + // parse (optional) generic params + std::vector> generic_params + = parse_generic_params_in_angles (); + + if (!skip_token (LEFT_PAREN)) + { + skip_after_semicolon (); + return nullptr; + } + + // parse parameters + std::vector function_params + = parse_named_function_params ( + [] (TokenId id) { return id == RIGHT_PAREN; }); + + if (!skip_token (RIGHT_PAREN)) + { + skip_after_semicolon (); + return nullptr; + } + + // parse (optional) return type + std::unique_ptr return_type = parse_function_return_type (); + + // parse (optional) where clause + AST::WhereClause where_clause = parse_where_clause (); + + if (!skip_token (SEMICOLON)) + { + // skip somewhere? + return nullptr; + } + + function_params.shrink_to_fit (); + + return std::unique_ptr ( + new AST::ExternalFunctionItem ( + std::move (ident), std::move (generic_params), std::move (return_type), + std::move (where_clause), std::move (function_params), std::move (vis), + std::move (outer_attrs), locus)); +} + // Parses a single extern block item (static or function declaration). template std::unique_ptr @@ -6031,112 +6186,9 @@ Parser::parse_external_item () has_mut, std::move (vis), std::move (outer_attrs), locus)); } - case FN_TOK: { - // parse extern function declaration item - // skip function token - lexer.skip_token (); - - // parse identifier - const_TokenPtr ident_tok = expect_token (IDENTIFIER); - if (ident_tok == nullptr) - { - skip_after_semicolon (); - return nullptr; - } - Identifier ident{ident_tok}; - - // parse (optional) generic params - std::vector> generic_params - = parse_generic_params_in_angles (); - - if (!skip_token (LEFT_PAREN)) - { - skip_after_semicolon (); - return nullptr; - } - - // parse parameters - std::vector function_params; - bool is_variadic = false; - AST::AttrVec variadic_attrs; - - const_TokenPtr t = lexer.peek_token (); - while (t->get_id () != RIGHT_PAREN) - { - AST::AttrVec maybe_variadic_attrs = parse_outer_attributes (); - if (lexer.peek_token ()->get_id () == ELLIPSIS) - { - // variadic - use attrs for this - lexer.skip_token (); - is_variadic = true; - variadic_attrs = std::move (maybe_variadic_attrs); - t = lexer.peek_token (); - - if (t->get_id () != RIGHT_PAREN) - { - Error error (t->get_locus (), - "expected right parentheses after variadic in " - "named function " - "parameters, found %qs", - t->get_token_description ()); - add_error (std::move (error)); - - skip_after_semicolon (); - return nullptr; - } - - break; - } - - AST::NamedFunctionParam param - = parse_named_function_param (std::move (maybe_variadic_attrs)); - if (param.is_error ()) - { - Error error (t->get_locus (), "could not parse named function " - "parameter in external function"); - add_error (std::move (error)); - - skip_after_semicolon (); - return nullptr; - } - function_params.push_back (std::move (param)); - - if (lexer.peek_token ()->get_id () != COMMA) - break; - - // skip comma - lexer.skip_token (); - t = lexer.peek_token (); - } - - if (!skip_token (RIGHT_PAREN)) - { - skip_after_semicolon (); - return nullptr; - } - - // parse (optional) return type - std::unique_ptr return_type = parse_function_return_type (); - - // parse (optional) where clause - AST::WhereClause where_clause = parse_where_clause (); - - if (!skip_token (SEMICOLON)) - { - // skip somewhere? - return nullptr; - } - - function_params.shrink_to_fit (); - - return std::unique_ptr ( - new AST::ExternalFunctionItem ( - std::move (ident), std::move (generic_params), - std::move (return_type), std::move (where_clause), - std::move (function_params), is_variadic, - std::move (variadic_attrs), std::move (vis), - std::move (outer_attrs), locus)); - } + case FN_TOK: + return parse_external_function_item (std::move (vis), + std::move (outer_attrs)); case TYPE: return parse_external_type_item (std::move (vis), std::move (outer_attrs)); @@ -6152,56 +6204,6 @@ Parser::parse_external_item () } } -/* Parses an extern block function param (with "pattern" being _ or an - * identifier). */ -template -AST::NamedFunctionParam -Parser::parse_named_function_param ( - AST::AttrVec outer_attrs) -{ - // parse identifier/_ - std::string name; - - const_TokenPtr t = lexer.peek_token (); - location_t name_location = t->get_locus (); - switch (t->get_id ()) - { - case IDENTIFIER: - name = t->get_str (); - lexer.skip_token (); - break; - case UNDERSCORE: - name = "_"; - lexer.skip_token (); - break; - default: - // this is not a function param, but not necessarily an error - return AST::NamedFunctionParam::create_error (); - } - - if (!skip_token (COLON)) - { - // skip after somewhere? - return AST::NamedFunctionParam::create_error (); - } - - // parse (required) type - std::unique_ptr param_type = parse_type (); - if (param_type == nullptr) - { - Error error ( - lexer.peek_token ()->get_locus (), - "could not parse param type in extern block function declaration"); - add_error (std::move (error)); - - skip_after_semicolon (); - return AST::NamedFunctionParam::create_error (); - } - - return AST::NamedFunctionParam (std::move (name), std::move (param_type), - std::move (outer_attrs), name_location); -} - // Parses a statement (will further disambiguate any statement). template std::unique_ptr diff --git a/gcc/rust/parse/rust-parse.h b/gcc/rust/parse/rust-parse.h index 6a69533b3f2b..e4322754c701 100644 --- a/gcc/rust/parse/rust-parse.h +++ b/gcc/rust/parse/rust-parse.h @@ -298,6 +298,13 @@ private: AST::Lifetime lifetime_from_token (const_TokenPtr tok); std::unique_ptr parse_external_type_item (AST::Visibility vis, AST::AttrVec outer_attrs); + std::unique_ptr + parse_external_function_item (AST::Visibility vis, AST::AttrVec outer_attrs); + AST::NamedFunctionParam parse_named_function_param (); + template + std::vector + parse_named_function_params (EndTokenPred is_end_token); + std::unique_ptr parse_type_alias (AST::Visibility vis, AST::AttrVec outer_attrs); std::unique_ptr parse_struct (AST::Visibility vis, @@ -338,8 +345,6 @@ private: AST::AttrVec outer_attrs); std::unique_ptr parse_extern_block (AST::Visibility vis, AST::AttrVec outer_attrs); - AST::NamedFunctionParam parse_named_function_param (AST::AttrVec outer_attrs - = AST::AttrVec ()); AST::Method parse_method (); // Expression-related (Pratt parsed) diff --git a/gcc/rust/resolve/rust-ast-resolve-item.cc b/gcc/rust/resolve/rust-ast-resolve-item.cc index 04fa72161751..8126f8370fbc 100644 --- a/gcc/rust/resolve/rust-ast-resolve-item.cc +++ b/gcc/rust/resolve/rust-ast-resolve-item.cc @@ -1115,9 +1115,8 @@ ResolveExternItem::visit (AST::ExternalFunctionItem &function) // we make a new scope so the names of parameters are resolved and shadowed // correctly for (auto ¶m : function.get_function_params ()) - { + if (!param.is_variadic ()) ResolveType::go (param.get_type ().get ()); - } // done resolver->get_name_scope ().pop (); diff --git a/gcc/rust/resolve/rust-early-name-resolver.cc b/gcc/rust/resolve/rust-early-name-resolver.cc index b9307c8612f2..4c78404f037c 100644 --- a/gcc/rust/resolve/rust-early-name-resolver.cc +++ b/gcc/rust/resolve/rust-early-name-resolver.cc @@ -872,7 +872,8 @@ EarlyNameResolver::visit (AST::ExternalFunctionItem &item) generic->accept_vis (*this); for (auto ¶m : item.get_function_params ()) - param.get_type ()->accept_vis (*this); + if (!param.is_variadic ()) + param.get_type ()->accept_vis (*this); if (item.has_return_type ()) item.get_return_type ()->accept_vis (*this);