c++: Fix up calls to static operator() or operator[] [PR107624]

Message ID Y3OF0UXlqmuibCUZ@tucnak
State Unresolved
Headers
Series c++: Fix up calls to static operator() or operator[] [PR107624] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Nov. 15, 2022, 12:28 p.m. UTC
  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

Jason Merrill Nov. 15, 2022, 9:49 p.m. UTC | #1
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
>
  

Patch

--- 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;
+};