On Thu, Jul 18, 2024 at 02:19:21PM -0400, Marek Polacek wrote: > On Wed, Jul 17, 2024 at 09:30:00PM -0700, Andi Kleen wrote: > > Implement a C23 clang compatible musttail attribute similar to the earlier > > C++ implementation in the C parser. > > > > gcc/c/ChangeLog: > > > > PR c/83324 > > * c-parser.cc (struct attr_state): Define with musttail_p. > > (c_parser_statement_after_labels): Handle [[musttail]]. > > (c_parser_std_attribute): Dito. > > (c_parser_handle_musttail): Dito. > > (c_parser_compound_statement_nostart): Dito. > > (c_parser_all_labels): Dito. > > (c_parser_statement): Dito. > > * c-tree.h (c_finish_return): Add musttail_p flag. > > * c-typeck.cc (c_finish_return): Handle musttail_p flag. > > --- > > gcc/c/c-parser.cc | 70 ++++++++++++++++++++++++++++++++++++++--------- > > gcc/c/c-tree.h | 2 +- > > gcc/c/c-typeck.cc | 7 +++-- > > 3 files changed, 63 insertions(+), 16 deletions(-) > > > > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc > > index 12c5ed5d92c7..a8848d01f21a 100644 > > --- a/gcc/c/c-parser.cc > > +++ b/gcc/c/c-parser.cc > > @@ -1621,6 +1621,12 @@ struct omp_for_parse_data { > > bool fail : 1; > > }; > > > > +struct attr_state > > +{ > > + /* True if we parsed a musttail attribute for return. */ > > + bool musttail_p; > > +}; > > + > > static bool c_parser_nth_token_starts_std_attributes (c_parser *, > > unsigned int); > > static tree c_parser_std_attribute_specifier_sequence (c_parser *); > > @@ -1665,7 +1671,7 @@ static location_t c_parser_compound_statement_nostart > > (c_parser *); > > static void c_parser_label (c_parser *, tree); > > static void c_parser_statement (c_parser *, bool *, location_t * = NULL); > > static void c_parser_statement_after_labels (c_parser *, bool *, > > - vec<tree> * = NULL); > > + vec<tree> * = NULL, attr_state = > > {}); > > Nit: the line seems to go over 80 columns.
Ok. > > || c_parser_next_token_is_keyword (parser, RID_DEFAULT) > > || (c_parser_next_token_is (parser, CPP_NAME) > > @@ -7346,7 +7384,10 @@ c_parser_all_labels (c_parser *parser) > > std_attrs = NULL; > > if ((have_std_attrs = c_parser_nth_token_starts_std_attributes > > (parser, > > 1))) > > - std_attrs = c_parser_std_attribute_specifier_sequence (parser); > > + { > > + std_attrs = c_parser_std_attribute_specifier_sequence (parser); > > + std_attrs = c_parser_handle_musttail (parser, std_attrs, attr); > > + } > > Thanks, I believe this addresses the testcase I mentioned earlier: > > struct str > { > int a, b; > }; > > struct str > cstruct (int x) > { > if (x < 10) > L: // <==== > [[gnu::musttail]] return cstruct (x + 1); > return ((struct str){ x, 0 }); > } > > but I didn't see that being tested in your testsuite patch; apologies if > I missed it. It wasn't there. I will add it. > > > tree > > -c_finish_return (location_t loc, tree retval, tree origtype) > > +c_finish_return (location_t loc, tree retval, tree origtype, bool > > musttail_p) > > { > > tree valtype = TREE_TYPE (TREE_TYPE (current_function_decl)), ret_stmt; > > bool no_warning = false; > > @@ -11742,6 +11743,8 @@ c_finish_return (location_t loc, tree retval, tree > > origtype) > > warning_at (xloc, 0, > > "function declared %<noreturn%> has a %<return%> statement"); > > > > + set_musttail_on_return (retval, xloc, musttail_p); > > + > > if (retval) > > { > > tree semantic_type = NULL_TREE; > > Is it deliberate that set_musttail_on_return is called outside the > if (retval) block? If it can be moved into it, set_musttail_on_return > can be simplified to assume that retval is always non-null. Yes it can be removed. Is the patchk ok with these changes? -Andi