c++: Fix up calls to static operator() or operator[] [PR107624]
Checks
Commit Message
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?
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.
Jakub
Comments
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
>
@@ -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 ();
@@ -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))
{
@@ -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 ();
+}
@@ -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
+}
@@ -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;
+};