Message ID | Zr5qw78QUDz4ks8/@tucnak |
---|---|
State | New |
Headers | show |
Series | c++: Allow standard attributes after closing square bracket in new-type-id [PR110345] | expand |
On 8/15/24 4:53 PM, Jakub Jelinek wrote: > Hi! > > For C++ 26 P2552R3 I went through all the spots (except modules) where > attribute-specifier-seq appears in the grammar and tried to construct > a testcase in all those spots, for now for [[deprecated]] attribute. > > The first thing I found is that we aren't parsing standard attributes in > noptr-new-declarator - https://eel.is/c++draft/expr.new#1 > > The following patch parses it there, for the non-outermost arrays > applies normally the attributes to the array type, for the outermost > where we just set *nelts and don't really build an array type just > warns that we ignore those attributes (or, do you think we should > just build an array type in that case and just take its element type?). It seems cleaner to just build an array type, and build_new_1 appears to already handle that. But I don't think there are currently any attributes that would be useful in this position, so this patch is fine with a FIXME if you don't want to mess with it. > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2024-08-15 Jakub Jelinek <jakub@redhat.com> > > PR c++/110345 > * parser.cc (make_array_declarator): Add STD_ATTRS argument, set > declarator->std_attributes to it. > (cp_parser_new_type_id): Warn on non-ignored std_attributes on the > array declarator which is being omitted. > (cp_parser_direct_new_declarator): Parse standard attributes after > closing square bracket, pass it to make_array_declarator. > (cp_parser_direct_declarator): Pass std_attrs to make_array_declarator > instead of setting declarator->std_attributes manually. > > * g++.dg/cpp0x/gen-attrs-80.C: New test. > * g++.dg/cpp0x/gen-attrs-81.C: New test. > > --- gcc/cp/parser.cc.jj 2024-08-15 14:56:29.007757215 +0200 > +++ gcc/cp/parser.cc 2024-08-15 16:28:18.607692746 +0200 > @@ -1689,7 +1689,7 @@ static cp_declarator *make_call_declarat > (cp_declarator *, tree, cp_cv_quals, cp_virt_specifiers, cp_ref_qualifier, > tree, tree, tree, tree, tree, location_t); > static cp_declarator *make_array_declarator > - (cp_declarator *, tree); > + (cp_declarator *, tree, tree); > static cp_declarator *make_pointer_declarator > (cp_cv_quals, cp_declarator *, tree); > static cp_declarator *make_reference_declarator > @@ -1904,10 +1904,11 @@ make_call_declarator (cp_declarator *tar > } > > /* Make a declarator for an array of BOUNDS elements, each of which is > - defined by ELEMENT. */ > + defined by ELEMENT. STD_ATTRS contains attributes that appertain to > + the array type. */ > > cp_declarator * > -make_array_declarator (cp_declarator *element, tree bounds) > +make_array_declarator (cp_declarator *element, tree bounds, tree std_attrs) > { > cp_declarator *declarator; > > @@ -1923,6 +1924,8 @@ make_array_declarator (cp_declarator *el > else > declarator->parameter_pack_p = false; > > + declarator->std_attributes = std_attrs; > + > return declarator; > } > > @@ -9784,6 +9787,12 @@ cp_parser_new_type_id (cp_parser* parser > if (*nelts == error_mark_node) > *nelts = integer_one_node; > > + if (*nelts > + && declarator->std_attributes > + && any_nonignored_attribute_p (declarator->std_attributes)) > + warning (OPT_Wattributes, "attributes ignored on outermost array " > + "type in new expression"); > + > if (*nelts == NULL_TREE) > /* Leave [] in the declarator. */; > else if (outer_declarator) > @@ -9838,8 +9847,8 @@ cp_parser_new_declarator_opt (cp_parser* > /* Parse a direct-new-declarator. > > direct-new-declarator: > - [ expression ] > - direct-new-declarator [constant-expression] > + [ expression ] attribute-specifier-seq [opt] > + direct-new-declarator [constant-expression] attribute-specifier-seq [opt] > > */ > > @@ -9886,8 +9895,9 @@ cp_parser_direct_new_declarator (cp_pars > /* Look for the closing `]'. */ > cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE); > > + tree attrs = cp_parser_std_attribute_spec_seq (parser); > /* Add this bound to the declarator. */ > - declarator = make_array_declarator (declarator, expression); > + declarator = make_array_declarator (declarator, expression, attrs); > > /* If the next token is not a `[', then there are no more > bounds. */ > @@ -24330,8 +24340,7 @@ cp_parser_direct_declarator (cp_parser* > } > > attrs = cp_parser_std_attribute_spec_seq (parser); > - declarator = make_array_declarator (declarator, bounds); > - declarator->std_attributes = attrs; > + declarator = make_array_declarator (declarator, bounds, attrs); > } > else if (first && dcl_kind != CP_PARSER_DECLARATOR_ABSTRACT) > { > --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-80.C.jj 2024-08-15 15:27:00.297061501 +0200 > +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-80.C 2024-08-15 15:27:19.610822213 +0200 > @@ -0,0 +1,10 @@ > +// { dg-do compile { target c++11 } } > + > +void > +foo (int n) > +{ > + auto a = new int [n] [[]]; > + auto b = new int [n] [[]] [42] [[]] [1] [[]]; > + delete[] b; > + delete[] a; > +} > --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-81.C.jj 2024-08-15 17:14:43.912228130 +0200 > +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-81.C 2024-08-15 17:19:01.738038053 +0200 > @@ -0,0 +1,11 @@ > +// { dg-do compile { target c++11 } } > + > +void > +foo (int n) > +{ > + auto a = new int [n] [[gnu::deprecated]]; // { dg-warning "attributes ignored on outermost array type in new expression" } > + auto b = new int [n] [[gnu::deprecated]] [42] [[]] [1] [[]]; // { dg-warning "attributes ignored on outermost array type in new expression" } > + auto c = new int [n] [[]] [42] [[gnu::deprecated]] [1] [[gnu::deprecated]]; > + delete[] b; > + delete[] a; > +} > > Jakub >
On Mon, Aug 19, 2024 at 05:18:14PM -0400, Jason Merrill wrote: > > The following patch parses it there, for the non-outermost arrays > > applies normally the attributes to the array type, for the outermost > > where we just set *nelts and don't really build an array type just > > warns that we ignore those attributes (or, do you think we should > > just build an array type in that case and just take its element type?). > > It seems cleaner to just build an array type, and build_new_1 appears to > already handle that. But I don't think there are currently any attributes > that would be useful in this position, so this patch is fine with a FIXME if > you don't want to mess with it. Unfortunately it isn't that easy to build the array, at least not by not stripping that one declarator, doing groktypename and then stripping the outermost array from it. The problem is that C++ pedantically doesn't support VLAs and the outermost new "array" is the only one which can be validly a VLA. So, we'd need to arrange for the VLA pedwarn to be disabled for the outermost array but enabled for the inner ones, as we want to pedwarn on new int[4][n] or new int[n][n] but not new int[n][4] Sure, we can just build the array manually without going through groktypename and immediately throw it away, but I mean if we'd actually never use that array for anything, it still might make sense to warn users that we are actually completely ignoring any (non-ignored) attribute there. Jakub
--- gcc/cp/parser.cc.jj 2024-08-15 14:56:29.007757215 +0200 +++ gcc/cp/parser.cc 2024-08-15 16:28:18.607692746 +0200 @@ -1689,7 +1689,7 @@ static cp_declarator *make_call_declarat (cp_declarator *, tree, cp_cv_quals, cp_virt_specifiers, cp_ref_qualifier, tree, tree, tree, tree, tree, location_t); static cp_declarator *make_array_declarator - (cp_declarator *, tree); + (cp_declarator *, tree, tree); static cp_declarator *make_pointer_declarator (cp_cv_quals, cp_declarator *, tree); static cp_declarator *make_reference_declarator @@ -1904,10 +1904,11 @@ make_call_declarator (cp_declarator *tar } /* Make a declarator for an array of BOUNDS elements, each of which is - defined by ELEMENT. */ + defined by ELEMENT. STD_ATTRS contains attributes that appertain to + the array type. */ cp_declarator * -make_array_declarator (cp_declarator *element, tree bounds) +make_array_declarator (cp_declarator *element, tree bounds, tree std_attrs) { cp_declarator *declarator; @@ -1923,6 +1924,8 @@ make_array_declarator (cp_declarator *el else declarator->parameter_pack_p = false; + declarator->std_attributes = std_attrs; + return declarator; } @@ -9784,6 +9787,12 @@ cp_parser_new_type_id (cp_parser* parser if (*nelts == error_mark_node) *nelts = integer_one_node; + if (*nelts + && declarator->std_attributes + && any_nonignored_attribute_p (declarator->std_attributes)) + warning (OPT_Wattributes, "attributes ignored on outermost array " + "type in new expression"); + if (*nelts == NULL_TREE) /* Leave [] in the declarator. */; else if (outer_declarator) @@ -9838,8 +9847,8 @@ cp_parser_new_declarator_opt (cp_parser* /* Parse a direct-new-declarator. direct-new-declarator: - [ expression ] - direct-new-declarator [constant-expression] + [ expression ] attribute-specifier-seq [opt] + direct-new-declarator [constant-expression] attribute-specifier-seq [opt] */ @@ -9886,8 +9895,9 @@ cp_parser_direct_new_declarator (cp_pars /* Look for the closing `]'. */ cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE); + tree attrs = cp_parser_std_attribute_spec_seq (parser); /* Add this bound to the declarator. */ - declarator = make_array_declarator (declarator, expression); + declarator = make_array_declarator (declarator, expression, attrs); /* If the next token is not a `[', then there are no more bounds. */ @@ -24330,8 +24340,7 @@ cp_parser_direct_declarator (cp_parser* } attrs = cp_parser_std_attribute_spec_seq (parser); - declarator = make_array_declarator (declarator, bounds); - declarator->std_attributes = attrs; + declarator = make_array_declarator (declarator, bounds, attrs); } else if (first && dcl_kind != CP_PARSER_DECLARATOR_ABSTRACT) { --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-80.C.jj 2024-08-15 15:27:00.297061501 +0200 +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-80.C 2024-08-15 15:27:19.610822213 +0200 @@ -0,0 +1,10 @@ +// { dg-do compile { target c++11 } } + +void +foo (int n) +{ + auto a = new int [n] [[]]; + auto b = new int [n] [[]] [42] [[]] [1] [[]]; + delete[] b; + delete[] a; +} --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-81.C.jj 2024-08-15 17:14:43.912228130 +0200 +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-81.C 2024-08-15 17:19:01.738038053 +0200 @@ -0,0 +1,11 @@ +// { dg-do compile { target c++11 } } + +void +foo (int n) +{ + auto a = new int [n] [[gnu::deprecated]]; // { dg-warning "attributes ignored on outermost array type in new expression" } + auto b = new int [n] [[gnu::deprecated]] [42] [[]] [1] [[]]; // { dg-warning "attributes ignored on outermost array type in new expression" } + auto c = new int [n] [[]] [42] [[gnu::deprecated]] [1] [[gnu::deprecated]]; + delete[] b; + delete[] a; +}