From cf958f8f168f695d49e29297ef9fb37f6efa5d0f Mon Sep 17 00:00:00 2001 From: Jakub Jelinek <jakub@redhat.com> Date: Wed, 16 Nov 2022 14:42:54 +0100 Subject: [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624] One thing that doesn't work properly is the same problem as I've filed yesterday for static operator() - PR107624 - that side-effects of the postfix-expression on which the call or subscript operator are applied are thrown away, I assume we have to add them into COMPOUND_EXPR somewhere after we find out that the we've chosen a static member function operator. 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. 2022-11-16 Jakub Jelinek <jakub@redhat.com> PR c++/107624 * call.cc (keep_unused_object_arg): New function. (build_op_call): Use it. (build_op_subscript): Likewise. (build_new_op): Similarly for ARRAY_REF. (build_new_method_call): Use it. * 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 | 66 ++++++++++++++----- gcc/cp/decl.cc | 4 ++ .../g++.dg/cpp23/static-operator-call4.C | 37 +++++++++++ gcc/testsuite/g++.dg/cpp23/subscript10.C | 46 +++++++++++++ gcc/testsuite/g++.dg/cpp23/subscript11.C | 11 ++++ 5 files changed, 148 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp23/static-operator-call4.C create mode 100644 gcc/testsuite/g++.dg/cpp23/subscript10.C create mode 100644 gcc/testsuite/g++.dg/cpp23/subscript11.C diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index dbb01ec6ee55..459e86b5f09f 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -5017,6 +5017,33 @@ build_operator_new_call (tree fnname, vec<tree, va_gc> **args, return ret; } +/* Evaluate side-effects from OBJ before evaluating call + to FN in RESULT expression. + This is for expressions of the form `obj->fn(...)' + where `fn' turns out to be a static member function and + `obj' needs to be evaluated. `fn' could be also static operator[] + or static operator(), in which cases the source expression + would be `obj[...]' or `obj(...)'. */ + +static tree +keep_unused_object_arg (tree result, tree obj, tree fn) +{ + if (result == NULL_TREE + || result == error_mark_node + || TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE + || !TREE_SIDE_EFFECTS (obj)) + return result; + + /* But avoid the implicit lvalue-rvalue conversion when `obj' is + volatile. */ + tree a = obj; + if (TREE_THIS_VOLATILE (a)) + a = build_this (a); + if (TREE_SIDE_EFFECTS (a)) + return build2 (COMPOUND_EXPR, TREE_TYPE (result), a, result); + return result; +} + /* Build a new call to operator(). This may change ARGS. */ tree @@ -5137,7 +5164,13 @@ build_op_call (tree obj, vec<tree, va_gc> **args, tsubst_flags_t complain) 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. */ + result = keep_unused_object_arg (result, obj, cand->fn); + } else { if (TREE_CODE (cand->fn) == FUNCTION_DECL) @@ -7046,6 +7079,12 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags, 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 = keep_unused_object_arg (result, arg1, cand->fn); } else { @@ -7302,6 +7341,11 @@ build_op_subscript (const op_location_t &loc, tree obj, /* 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. */ + result = keep_unused_object_arg (result, obj, cand->fn); } else gcc_unreachable (); @@ -11494,21 +11538,11 @@ build_new_method_call (tree instance, tree fns, vec<tree, va_gc> **args, /* In an expression of the form `a->f()' where `f' turns out to be a static member function, `a' is none-the-less evaluated. */ - if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE - && !is_dummy_object (instance) - && TREE_SIDE_EFFECTS (instance)) - { - /* But avoid the implicit lvalue-rvalue conversion when 'a' - is volatile. */ - tree a = instance; - if (TREE_THIS_VOLATILE (a)) - a = build_this (a); - if (TREE_SIDE_EFFECTS (a)) - call = build2 (COMPOUND_EXPR, TREE_TYPE (call), a, call); - } - else if (call != error_mark_node - && DECL_DESTRUCTOR_P (cand->fn) - && !VOID_TYPE_P (TREE_TYPE (call))) + if (!is_dummy_object (instance)) + call = keep_unused_object_arg (call, instance, fn); + if (call != error_mark_node + && DECL_DESTRUCTOR_P (cand->fn) + && !VOID_TYPE_P (TREE_TYPE (call))) /* An explicit call of the form "x->~X()" has type "void". However, on platforms where destructors return "this" (i.e., those where diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 0234919ea572..d28889ed8659 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -15404,6 +15404,10 @@ grok_op_properties (tree decl, bool complain) 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)) { diff --git a/gcc/testsuite/g++.dg/cpp23/static-operator-call4.C b/gcc/testsuite/g++.dg/cpp23/static-operator-call4.C new file mode 100644 index 000000000000..83ccde3b9c5b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp23/static-operator-call4.C @@ -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 (); +} diff --git a/gcc/testsuite/g++.dg/cpp23/subscript10.C b/gcc/testsuite/g++.dg/cpp23/subscript10.C new file mode 100644 index 000000000000..96e5e1f3ed29 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp23/subscript10.C @@ -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 +} diff --git a/gcc/testsuite/g++.dg/cpp23/subscript11.C b/gcc/testsuite/g++.dg/cpp23/subscript11.C new file mode 100644 index 000000000000..990a70327b24 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp23/subscript11.C @@ -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; +}; -- GitLab