| Message ID | Y3OF0UXlqmuibCUZ@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Fix up calls to static operator() or operator[] [PR107624] | expand |
On 11/15/22 02:28, Jakub Jelinek wrote: > Hi! > > On Mon, Nov 14, 2022 at 06:29:44PM -0500, Jason Merrill wrote: >> Indeed. The code in build_new_method_call for this case has the comment >> >> /* In an expression of the form `a->f()' where `f' turns >> out to be a static member function, `a' is >> none-the-less evaluated. */ > > Had to tweak 3 spots for this. Furthermore, found that if in non-pedantic > C++20 compilation static operator[] is accepted, we required that it has 2 > arguments, I think it is better to require exactly one because that case > is the only one that will actually work in C++20 and older. > > Lightly tested so far, ok for trunk if it passes bootstrap/regtest? > > Or do you want to outline the > if (result != error_mark_node > && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE > && TREE_SIDE_EFFECTS (obj)) > { > /* But avoid the implicit lvalue-rvalue conversion when 'a' > is volatile. */ > tree a = obj; > if (TREE_THIS_VOLATILE (a)) > a = build_this (a); > if (TREE_SIDE_EFFECTS (a)) > result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); > } > part that is now repeated 4 times to some helper function? If yes, > any suggestion on a good name? Please. Maybe keep_unused_object_arg? > 2022-11-15 Jakub Jelinek <jakub@redhat.com> > > PR c++/107624 > * call.cc (build_op_call): If obj has side-effects > and operator() is static member function, return COMPOUND_EXPR > with the obj side-effects other than reading from volatile > object. > (build_op_subscript): Likewise. > (build_new_op): Similarly for ARRAY_REF, just for arg1 rather than > obj. > * decl.cc (grok_op_properties): For C++20 and earlier, if operator[] > is static member function, require exactly one parameter rather than > exactly two parameters. > > * g++.dg/cpp23/static-operator-call4.C: New test. > * g++.dg/cpp23/subscript10.C: New test. > * g++.dg/cpp23/subscript11.C: New test. > > --- gcc/cp/call.cc.jj 2022-11-15 07:59:57.337231337 +0100 > +++ gcc/cp/call.cc 2022-11-15 13:02:33.369531156 +0100 > @@ -5137,7 +5137,24 @@ build_op_call (tree obj, vec<tree, va_gc > else if (TREE_CODE (cand->fn) == FUNCTION_DECL > && DECL_OVERLOADED_OPERATOR_P (cand->fn) > && DECL_OVERLOADED_OPERATOR_IS (cand->fn, CALL_EXPR)) > - result = build_over_call (cand, LOOKUP_NORMAL, complain); > + { > + result = build_over_call (cand, LOOKUP_NORMAL, complain); > + /* In an expression of the form `a()' where cand->fn > + which is operator() turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + if (result != error_mark_node > + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE > + && TREE_SIDE_EFFECTS (obj)) > + { > + /* But avoid the implicit lvalue-rvalue conversion when 'a' > + is volatile. */ > + tree a = obj; > + if (TREE_THIS_VOLATILE (a)) > + a = build_this (a); > + if (TREE_SIDE_EFFECTS (a)) > + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); > + } > + } > else > { > if (TREE_CODE (cand->fn) == FUNCTION_DECL) > @@ -7046,6 +7063,24 @@ build_new_op (const op_location_t &loc, > gcc_unreachable (); > } > } > + > + /* In an expression of the form `a[]' where cand->fn > + which is operator[] turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + if (code == ARRAY_REF > + && result > + && result != error_mark_node > + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE > + && TREE_SIDE_EFFECTS (arg1)) > + { > + /* But avoid the implicit lvalue-rvalue conversion when 'a' > + is volatile. */ > + tree a = arg1; > + if (TREE_THIS_VOLATILE (a)) > + a = build_this (a); > + if (TREE_SIDE_EFFECTS (a)) > + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); > + } > } > else > { > @@ -7302,6 +7337,24 @@ build_op_subscript (const op_location_t > /* Specify evaluation order as per P0145R2. */ > CALL_EXPR_ORDERED_ARGS (call) = op_is_ordered (ARRAY_REF) == 1; > } > + > + /* In an expression of the form `a[]' where cand->fn > + which is operator[] turns out to be a static member function, > + `a' is none-the-less evaluated. */ > + if (result > + && result != error_mark_node > + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE > + && TREE_SIDE_EFFECTS (obj)) > + { > + /* But avoid the implicit lvalue-rvalue conversion when 'a' > + is volatile. */ > + tree a = obj; > + if (TREE_THIS_VOLATILE (a)) > + a = build_this (a); > + if (TREE_SIDE_EFFECTS (a)) > + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); > + } > + > } > else > gcc_unreachable (); > --- gcc/cp/decl.cc.jj 2022-11-15 07:59:57.321231551 +0100 > +++ gcc/cp/decl.cc 2022-11-15 13:13:51.814356965 +0100 > @@ -15400,6 +15400,10 @@ grok_op_properties (tree decl, bool comp > pedwarn (loc, OPT_Wc__23_extensions, "%qD may be a static member " > "function only with %<-std=c++23%> or %<-std=gnu++23%>", > decl); > + if (operator_code == ARRAY_REF) > + /* static operator[] should have exactly one argument > + for C++20 and earlier, so that it isn't multidimensional. */ > + op_flags = OVL_OP_FLAG_UNARY; > } > else if (DECL_STATIC_FUNCTION_P (decl)) > { > --- gcc/testsuite/g++.dg/cpp23/static-operator-call4.C.jj 2022-11-15 11:26:28.187511136 +0100 > +++ gcc/testsuite/g++.dg/cpp23/static-operator-call4.C 2022-11-15 11:27:19.684814244 +0100 > @@ -0,0 +1,37 @@ > +// PR c++/107624 > +// { dg-do run { target c++11 } } > +// { dg-options "" } > + > +int n[3]; > +struct S { > + static void operator() (int x) { n[0] |= (1 << x); } // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + static void baz (int x) { n[1] |= (1 << x); } > + int s; > +}; > +volatile S s[2]; > + > +S & > +foo (int x) > +{ > + static S t; > + n[2] |= (1 << x); > + return t; > +} > + > +int > +main () > +{ > + int i = 0; > + foo (0) (0); > + if (n[0] != 1 || n[1] || n[2] != 1) > + __builtin_abort (); > + foo (1).baz (1); > + if (n[0] != 1 || n[1] != 2 || n[2] != 3) > + __builtin_abort (); > + s[i++] (2); > + if (i != 1 || n[0] != 5 || n[1] != 2 || n[2] != 3) > + __builtin_abort (); > + s[--i].baz (3); > + if (i != 0 || n[0] != 5 || n[1] != 10 || n[2] != 3) > + __builtin_abort (); > +} > --- gcc/testsuite/g++.dg/cpp23/subscript10.C.jj 2022-11-15 11:27:48.810420112 +0100 > +++ gcc/testsuite/g++.dg/cpp23/subscript10.C 2022-11-15 13:07:02.974885454 +0100 > @@ -0,0 +1,46 @@ > +// PR c++/107624 > +// { dg-do run { target c++11 } } > +// { dg-options "" } > + > +int n[3]; > +struct S { > + static int &operator[] (int x) { n[0] |= (1 << x); return n[2]; } // { dg-warning "may be a static member function only with" "" { target c++20_down } } > +#if __cpp_multidimensional_subscript >= 202211L > + static int &operator[] () { n[0] |= (1 << 8); return n[2]; } > + static int &operator[] (int y, int z, int w) { n[0] |= (1 << y) | (1 << z) | (1 << w); return n[2]; } > +#endif > + int s; > +}; > +volatile S s[2]; > + > +S & > +foo (int x) > +{ > + static S t; > + n[1] |= (1 << x); > + return t; > +} > + > +int > +main () > +{ > + int i = 0; > + foo (0) [0]++; > + if (n[0] != 1 || n[1] != 1 || n[2] != 1) > + __builtin_abort (); > + s[i++][2]++; > + if (i != 1 || n[0] != 5 || n[1] != 1 || n[2] != 2) > + __builtin_abort (); > +#if __cpp_multidimensional_subscript >= 202211L > + foo (3) []++; > + if (n[0] != 261 || n[1] != 9 || n[2] != 3) > + __builtin_abort (); > + int y = 10; > + int z = 10; > + int w = 13; > + foo (4) [y++, ++z, w++]++; > + if (n[0] != 11525 || n[1] != 25 || n[2] != 4 > + || y != 11 || z != 11 || w != 14) > + __builtin_abort (); > +#endif > +} > --- gcc/testsuite/g++.dg/cpp23/subscript11.C.jj 2022-11-15 13:14:50.938557460 +0100 > +++ gcc/testsuite/g++.dg/cpp23/subscript11.C 2022-11-15 13:19:20.681910074 +0100 > @@ -0,0 +1,11 @@ > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +struct S { > + static int &operator[] (int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + static int &operator[] (); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + // { dg-error "'static int& S::operator\\\[\\\]\\\(\\\)' must have exactly one argument" "" { target c++20_down } .-1 } > + static int &operator[] (int, int, int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } > + // { dg-error "'static int& S::operator\\\[\\\]\\\(int, int, int\\\)' must have exactly one argument" "" { target c++20_down } .-1 } > + int s; > +}; > > > Jakub >
--- gcc/cp/call.cc.jj 2022-11-15 07:59:57.337231337 +0100 +++ gcc/cp/call.cc 2022-11-15 13:02:33.369531156 +0100 @@ -5137,7 +5137,24 @@ build_op_call (tree obj, vec<tree, va_gc else if (TREE_CODE (cand->fn) == FUNCTION_DECL && DECL_OVERLOADED_OPERATOR_P (cand->fn) && DECL_OVERLOADED_OPERATOR_IS (cand->fn, CALL_EXPR)) - result = build_over_call (cand, LOOKUP_NORMAL, complain); + { + result = build_over_call (cand, LOOKUP_NORMAL, complain); + /* In an expression of the form `a()' where cand->fn + which is operator() turns out to be a static member function, + `a' is none-the-less evaluated. */ + if (result != error_mark_node + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE + && TREE_SIDE_EFFECTS (obj)) + { + /* But avoid the implicit lvalue-rvalue conversion when 'a' + is volatile. */ + tree a = obj; + if (TREE_THIS_VOLATILE (a)) + a = build_this (a); + if (TREE_SIDE_EFFECTS (a)) + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); + } + } else { if (TREE_CODE (cand->fn) == FUNCTION_DECL) @@ -7046,6 +7063,24 @@ build_new_op (const op_location_t &loc, gcc_unreachable (); } } + + /* In an expression of the form `a[]' where cand->fn + which is operator[] turns out to be a static member function, + `a' is none-the-less evaluated. */ + if (code == ARRAY_REF + && result + && result != error_mark_node + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE + && TREE_SIDE_EFFECTS (arg1)) + { + /* But avoid the implicit lvalue-rvalue conversion when 'a' + is volatile. */ + tree a = arg1; + if (TREE_THIS_VOLATILE (a)) + a = build_this (a); + if (TREE_SIDE_EFFECTS (a)) + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); + } } else { @@ -7302,6 +7337,24 @@ build_op_subscript (const op_location_t /* Specify evaluation order as per P0145R2. */ CALL_EXPR_ORDERED_ARGS (call) = op_is_ordered (ARRAY_REF) == 1; } + + /* In an expression of the form `a[]' where cand->fn + which is operator[] turns out to be a static member function, + `a' is none-the-less evaluated. */ + if (result + && result != error_mark_node + && TREE_CODE (TREE_TYPE (cand->fn)) != METHOD_TYPE + && TREE_SIDE_EFFECTS (obj)) + { + /* But avoid the implicit lvalue-rvalue conversion when 'a' + is volatile. */ + tree a = obj; + if (TREE_THIS_VOLATILE (a)) + a = build_this (a); + if (TREE_SIDE_EFFECTS (a)) + result = build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); + } + } else gcc_unreachable (); --- gcc/cp/decl.cc.jj 2022-11-15 07:59:57.321231551 +0100 +++ gcc/cp/decl.cc 2022-11-15 13:13:51.814356965 +0100 @@ -15400,6 +15400,10 @@ grok_op_properties (tree decl, bool comp pedwarn (loc, OPT_Wc__23_extensions, "%qD may be a static member " "function only with %<-std=c++23%> or %<-std=gnu++23%>", decl); + if (operator_code == ARRAY_REF) + /* static operator[] should have exactly one argument + for C++20 and earlier, so that it isn't multidimensional. */ + op_flags = OVL_OP_FLAG_UNARY; } else if (DECL_STATIC_FUNCTION_P (decl)) { --- gcc/testsuite/g++.dg/cpp23/static-operator-call4.C.jj 2022-11-15 11:26:28.187511136 +0100 +++ gcc/testsuite/g++.dg/cpp23/static-operator-call4.C 2022-11-15 11:27:19.684814244 +0100 @@ -0,0 +1,37 @@ +// PR c++/107624 +// { dg-do run { target c++11 } } +// { dg-options "" } + +int n[3]; +struct S { + static void operator() (int x) { n[0] |= (1 << x); } // { dg-warning "may be a static member function only with" "" { target c++20_down } } + static void baz (int x) { n[1] |= (1 << x); } + int s; +}; +volatile S s[2]; + +S & +foo (int x) +{ + static S t; + n[2] |= (1 << x); + return t; +} + +int +main () +{ + int i = 0; + foo (0) (0); + if (n[0] != 1 || n[1] || n[2] != 1) + __builtin_abort (); + foo (1).baz (1); + if (n[0] != 1 || n[1] != 2 || n[2] != 3) + __builtin_abort (); + s[i++] (2); + if (i != 1 || n[0] != 5 || n[1] != 2 || n[2] != 3) + __builtin_abort (); + s[--i].baz (3); + if (i != 0 || n[0] != 5 || n[1] != 10 || n[2] != 3) + __builtin_abort (); +} --- gcc/testsuite/g++.dg/cpp23/subscript10.C.jj 2022-11-15 11:27:48.810420112 +0100 +++ gcc/testsuite/g++.dg/cpp23/subscript10.C 2022-11-15 13:07:02.974885454 +0100 @@ -0,0 +1,46 @@ +// PR c++/107624 +// { dg-do run { target c++11 } } +// { dg-options "" } + +int n[3]; +struct S { + static int &operator[] (int x) { n[0] |= (1 << x); return n[2]; } // { dg-warning "may be a static member function only with" "" { target c++20_down } } +#if __cpp_multidimensional_subscript >= 202211L + static int &operator[] () { n[0] |= (1 << 8); return n[2]; } + static int &operator[] (int y, int z, int w) { n[0] |= (1 << y) | (1 << z) | (1 << w); return n[2]; } +#endif + int s; +}; +volatile S s[2]; + +S & +foo (int x) +{ + static S t; + n[1] |= (1 << x); + return t; +} + +int +main () +{ + int i = 0; + foo (0) [0]++; + if (n[0] != 1 || n[1] != 1 || n[2] != 1) + __builtin_abort (); + s[i++][2]++; + if (i != 1 || n[0] != 5 || n[1] != 1 || n[2] != 2) + __builtin_abort (); +#if __cpp_multidimensional_subscript >= 202211L + foo (3) []++; + if (n[0] != 261 || n[1] != 9 || n[2] != 3) + __builtin_abort (); + int y = 10; + int z = 10; + int w = 13; + foo (4) [y++, ++z, w++]++; + if (n[0] != 11525 || n[1] != 25 || n[2] != 4 + || y != 11 || z != 11 || w != 14) + __builtin_abort (); +#endif +} --- gcc/testsuite/g++.dg/cpp23/subscript11.C.jj 2022-11-15 13:14:50.938557460 +0100 +++ gcc/testsuite/g++.dg/cpp23/subscript11.C 2022-11-15 13:19:20.681910074 +0100 @@ -0,0 +1,11 @@ +// { dg-do compile { target c++11 } } +// { dg-options "" } + +struct S { + static int &operator[] (int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } + static int &operator[] (); // { dg-warning "may be a static member function only with" "" { target c++20_down } } + // { dg-error "'static int& S::operator\\\[\\\]\\\(\\\)' must have exactly one argument" "" { target c++20_down } .-1 } + static int &operator[] (int, int, int); // { dg-warning "may be a static member function only with" "" { target c++20_down } } + // { dg-error "'static int& S::operator\\\[\\\]\\\(int, int, int\\\)' must have exactly one argument" "" { target c++20_down } .-1 } + int s; +};