public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Implement C++23 P2589R1 - - static operator[]
@ 2022-11-11  7:40 Jakub Jelinek
  2022-11-14 23:29 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-11-11  7:40 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

As stage1 is very close, here is a patch that implements the static
operator[] paper.
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.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
provided the paper gets voted into C++23?

2022-11-11  Jakub Jelinek  <jakub@redhat.com>

gcc/c-family/
	* c-cppbuiltin.cc (c_cpp_builtins): Bump C++23
	__cpp_multidimensional_subscript macro value to 202211L.
gcc/cp/
	* decl.cc (grok_op_properties): Implement C++23 P2589R1
	- static operator[].  Handle operator[] similarly to operator()
	- allow static member functions, but pedwarn on it for C++20 and
	older.  Unlike operator(), perform rest of checks on it though for
	C++20.
	* call.cc (add_operator_candidates): For operator[] with class
	typed first parameter, pass that parameter as first_arg and
	an adjusted arglist without that parameter.
gcc/testsuite/
	* g++.dg/cpp23/subscript9.C: New test.
	* g++.dg/cpp23/feat-cxx2b.C: Expect a newer
	__cpp_multidimensional_subscript value.
	* g++.old-deja/g++.bugs/900210_10.C: Don't expect an error
	for C++23 or later.
	
--- gcc/c-family/c-cppbuiltin.cc.jj	2022-10-14 09:35:56.182990495 +0200
+++ gcc/c-family/c-cppbuiltin.cc	2022-11-10 22:29:12.539832741 +0100
@@ -1075,7 +1075,7 @@ c_cpp_builtins (cpp_reader *pfile)
 	  cpp_define (pfile, "__cpp_size_t_suffix=202011L");
 	  cpp_define (pfile, "__cpp_if_consteval=202106L");
 	  cpp_define (pfile, "__cpp_constexpr=202110L");
-	  cpp_define (pfile, "__cpp_multidimensional_subscript=202110L");
+	  cpp_define (pfile, "__cpp_multidimensional_subscript=202211L");
 	  cpp_define (pfile, "__cpp_named_character_escapes=202207L");
 	  cpp_define (pfile, "__cpp_static_call_operator=202207L");
 	  cpp_define (pfile, "__cpp_implicit_move=202207L");
--- gcc/cp/decl.cc.jj	2022-11-08 09:54:37.313400209 +0100
+++ gcc/cp/decl.cc	2022-11-10 21:26:06.891359343 +0100
@@ -15377,7 +15377,15 @@ grok_op_properties (tree decl, bool comp
      an enumeration, or a reference to an enumeration.  13.4.0.6 */
   if (! methodp || DECL_STATIC_FUNCTION_P (decl))
     {
-      if (operator_code == CALL_EXPR)
+      if (operator_code == TYPE_EXPR
+	  || operator_code == COMPONENT_REF
+	  || operator_code == NOP_EXPR)
+	{
+	  error_at (loc, "%qD must be a non-static member function", decl);
+	  return false;
+	}
+
+      if (operator_code == CALL_EXPR || operator_code == ARRAY_REF)
 	{
 	  if (! DECL_STATIC_FUNCTION_P (decl))
 	    {
@@ -15386,52 +15394,41 @@ grok_op_properties (tree decl, bool comp
 	    }
 	  if (cxx_dialect < cxx23
 	      /* For lambdas we diagnose static lambda specifier elsewhere.  */
-	      && ! LAMBDA_FUNCTION_P (decl)
+	      && (operator_code == ARRAY_REF || ! LAMBDA_FUNCTION_P (decl))
 	      /* For instantiations, we have diagnosed this already.  */
 	      && ! DECL_USE_TEMPLATE (decl))
 	    pedwarn (loc, OPT_Wc__23_extensions, "%qD may be a static member "
-	      "function only with %<-std=c++23%> or %<-std=gnu++23%>", decl);
-	  /* There are no further restrictions on the arguments to an
-	     overloaded "operator ()".  */
-	  return true;
-	}
-      if (operator_code == TYPE_EXPR
-	  || operator_code == COMPONENT_REF
-	  || operator_code == ARRAY_REF
-	  || operator_code == NOP_EXPR)
-	{
-	  error_at (loc, "%qD must be a non-static member function", decl);
-	  return false;
+		     "function only with %<-std=c++23%> or %<-std=gnu++23%>",
+		     decl);
 	}
-
-      if (DECL_STATIC_FUNCTION_P (decl))
+      else if (DECL_STATIC_FUNCTION_P (decl))
 	{
 	  error_at (loc, "%qD must be either a non-static member "
 		    "function or a non-member function", decl);
 	  return false;
 	}
-
-      for (tree arg = argtypes; ; arg = TREE_CHAIN (arg))
-	{
-	  if (!arg || arg == void_list_node)
-	    {
-	      if (complain)
-		error_at(loc, "%qD must have an argument of class or "
-			 "enumerated type", decl);
-	      return false;
-	    }
+      else
+	for (tree arg = argtypes; ; arg = TREE_CHAIN (arg))
+	  {
+	    if (!arg || arg == void_list_node)
+	      {
+		if (complain)
+		  error_at (loc, "%qD must have an argument of class or "
+			    "enumerated type", decl);
+		return false;
+	      }
       
-	  tree type = non_reference (TREE_VALUE (arg));
-	  if (type == error_mark_node)
-	    return false;
-	  
-	  /* MAYBE_CLASS_TYPE_P, rather than CLASS_TYPE_P, is used
-	     because these checks are performed even on template
-	     functions.  */
-	  if (MAYBE_CLASS_TYPE_P (type)
-	      || TREE_CODE (type) == ENUMERAL_TYPE)
-	    break;
-	}
+	    tree type = non_reference (TREE_VALUE (arg));
+	    if (type == error_mark_node)
+	      return false;
+
+	    /* MAYBE_CLASS_TYPE_P, rather than CLASS_TYPE_P, is used
+	       because these checks are performed even on template
+	       functions.  */
+	    if (MAYBE_CLASS_TYPE_P (type)
+		|| TREE_CODE (type) == ENUMERAL_TYPE)
+	      break;
+	  }
     }
 
   if (operator_code == CALL_EXPR)
--- gcc/cp/call.cc.jj	2022-11-08 09:54:37.283400616 +0100
+++ gcc/cp/call.cc	2022-11-10 22:26:38.096936491 +0100
@@ -6589,12 +6589,36 @@ add_operator_candidates (z_candidate **c
       if (fns == error_mark_node)
 	return error_mark_node;
       if (fns)
-	add_candidates (BASELINK_FUNCTIONS (fns),
-			NULL_TREE, arglist, NULL_TREE,
-			NULL_TREE, false,
-			BASELINK_BINFO (fns),
-			BASELINK_ACCESS_BINFO (fns),
-			flags, candidates, complain);
+	{
+	  if (code == ARRAY_REF)
+	    {
+	      vec<tree,va_gc> *restlist = make_tree_vector ();
+	      for (unsigned i = 1; i < nargs; ++i)
+		vec_safe_push (restlist, (*arglist)[i]);
+	      z_candidate *save_cand = *candidates;
+	      add_candidates (BASELINK_FUNCTIONS (fns),
+			      (*arglist)[0], restlist, NULL_TREE,
+			      NULL_TREE, false,
+			      BASELINK_BINFO (fns),
+			      BASELINK_ACCESS_BINFO (fns),
+			      flags, candidates, complain);
+	      /* Release the vec if we didn't add a candidate that uses it.  */
+	      for (z_candidate *c = *candidates; c != save_cand; c = c->next)
+		if (c->args == restlist)
+		  {
+		    restlist = NULL;
+		    break;
+		  }
+	      release_tree_vector (restlist);
+	    }
+	  else
+	    add_candidates (BASELINK_FUNCTIONS (fns),
+			    NULL_TREE, arglist, NULL_TREE,
+			    NULL_TREE, false,
+			    BASELINK_BINFO (fns),
+			    BASELINK_ACCESS_BINFO (fns),
+			    flags, candidates, complain);
+	}
     }
   /* Per [over.match.oper]3.2, if no operand has a class type, then
      only non-member functions that have type T1 or reference to
--- gcc/testsuite/g++.dg/cpp23/subscript9.C.jj	2022-11-10 21:06:58.155974655 +0100
+++ gcc/testsuite/g++.dg/cpp23/subscript9.C	2022-11-11 08:26:26.502445107 +0100
@@ -0,0 +1,29 @@
+// P2589R1
+// { dg-do run { target c++23 } }
+
+extern "C" void abort ();
+
+struct S
+{
+  S () {};
+  static int &operator[] () { return a[0]; }
+  static int &operator[] (int x) { return a[x]; }
+  static int &operator[] (int x, long y) { return a[x + y * 8]; }
+  static int a[64];
+};
+int S::a[64];
+
+int
+main ()
+{
+  S s;
+  for (int i = 0; i < 64; i++)
+    s.a[i] = 64 - i;
+  if (s[] != 64 || s[3] != 61 || s[4, 5] != 20)
+    abort ();
+  s[]++;
+  s[42]++;
+  ++s[3, 2];
+  if (s.a[0] != 65 || s.a[42] != 23 || s.a[19] != 46)
+    abort ();
+}
--- gcc/testsuite/g++.dg/cpp23/feat-cxx2b.C.jj	2022-10-06 08:56:28.785125656 +0200
+++ gcc/testsuite/g++.dg/cpp23/feat-cxx2b.C	2022-11-10 22:31:49.187698955 +0100
@@ -556,8 +556,8 @@
 
 #ifndef __cpp_multidimensional_subscript
 #  error "__cpp_multidimensional_subscript"
-#elif __cpp_multidimensional_subscript != 202110
-#  error "__cpp_multidimensional_subscript != 202110"
+#elif __cpp_multidimensional_subscript != 202211
+#  error "__cpp_multidimensional_subscript != 202211"
 #endif
 
 #ifndef __cpp_named_character_escapes
--- gcc/testsuite/g++.old-deja/g++.bugs/900210_10.C.jj	2020-01-14 20:02:47.021606269 +0100
+++ gcc/testsuite/g++.old-deja/g++.bugs/900210_10.C	2022-11-11 01:41:13.840971578 +0100
@@ -9,7 +9,7 @@
 // keywords: operator[], static function members
 
 struct struct0 {
-  static int operator[] ();		/* { dg-error "" } */
+  static int operator[] ();		/* { dg-error "" "" { target c++20_down } } */
 };
 
 int main () { return 0; }

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: Implement C++23 P2589R1 - - static operator[]
  2022-11-11  7:40 [PATCH] c++: Implement C++23 P2589R1 - - static operator[] Jakub Jelinek
@ 2022-11-14 23:29 ` Jason Merrill
  2022-11-15 12:28   ` [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624] Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-11-14 23:29 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 11/10/22 21:40, Jakub Jelinek wrote:
> Hi!
> 
> As stage1 is very close, here is a patch that implements the static
> operator[] paper.
> 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.

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.  */

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> provided the paper gets voted into C++23?

OK.

> 2022-11-11  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/c-family/
> 	* c-cppbuiltin.cc (c_cpp_builtins): Bump C++23
> 	__cpp_multidimensional_subscript macro value to 202211L.
> gcc/cp/
> 	* decl.cc (grok_op_properties): Implement C++23 P2589R1
> 	- static operator[].  Handle operator[] similarly to operator()
> 	- allow static member functions, but pedwarn on it for C++20 and
> 	older.  Unlike operator(), perform rest of checks on it though for
> 	C++20.
> 	* call.cc (add_operator_candidates): For operator[] with class
> 	typed first parameter, pass that parameter as first_arg and
> 	an adjusted arglist without that parameter.
> gcc/testsuite/
> 	* g++.dg/cpp23/subscript9.C: New test.
> 	* g++.dg/cpp23/feat-cxx2b.C: Expect a newer
> 	__cpp_multidimensional_subscript value.
> 	* g++.old-deja/g++.bugs/900210_10.C: Don't expect an error
> 	for C++23 or later.
> 	
> --- gcc/c-family/c-cppbuiltin.cc.jj	2022-10-14 09:35:56.182990495 +0200
> +++ gcc/c-family/c-cppbuiltin.cc	2022-11-10 22:29:12.539832741 +0100
> @@ -1075,7 +1075,7 @@ c_cpp_builtins (cpp_reader *pfile)
>   	  cpp_define (pfile, "__cpp_size_t_suffix=202011L");
>   	  cpp_define (pfile, "__cpp_if_consteval=202106L");
>   	  cpp_define (pfile, "__cpp_constexpr=202110L");
> -	  cpp_define (pfile, "__cpp_multidimensional_subscript=202110L");
> +	  cpp_define (pfile, "__cpp_multidimensional_subscript=202211L");
>   	  cpp_define (pfile, "__cpp_named_character_escapes=202207L");
>   	  cpp_define (pfile, "__cpp_static_call_operator=202207L");
>   	  cpp_define (pfile, "__cpp_implicit_move=202207L");
> --- gcc/cp/decl.cc.jj	2022-11-08 09:54:37.313400209 +0100
> +++ gcc/cp/decl.cc	2022-11-10 21:26:06.891359343 +0100
> @@ -15377,7 +15377,15 @@ grok_op_properties (tree decl, bool comp
>        an enumeration, or a reference to an enumeration.  13.4.0.6 */
>     if (! methodp || DECL_STATIC_FUNCTION_P (decl))
>       {
> -      if (operator_code == CALL_EXPR)
> +      if (operator_code == TYPE_EXPR
> +	  || operator_code == COMPONENT_REF
> +	  || operator_code == NOP_EXPR)
> +	{
> +	  error_at (loc, "%qD must be a non-static member function", decl);
> +	  return false;
> +	}
> +
> +      if (operator_code == CALL_EXPR || operator_code == ARRAY_REF)
>   	{
>   	  if (! DECL_STATIC_FUNCTION_P (decl))
>   	    {
> @@ -15386,52 +15394,41 @@ grok_op_properties (tree decl, bool comp
>   	    }
>   	  if (cxx_dialect < cxx23
>   	      /* For lambdas we diagnose static lambda specifier elsewhere.  */
> -	      && ! LAMBDA_FUNCTION_P (decl)
> +	      && (operator_code == ARRAY_REF || ! LAMBDA_FUNCTION_P (decl))
>   	      /* For instantiations, we have diagnosed this already.  */
>   	      && ! DECL_USE_TEMPLATE (decl))
>   	    pedwarn (loc, OPT_Wc__23_extensions, "%qD may be a static member "
> -	      "function only with %<-std=c++23%> or %<-std=gnu++23%>", decl);
> -	  /* There are no further restrictions on the arguments to an
> -	     overloaded "operator ()".  */
> -	  return true;
> -	}
> -      if (operator_code == TYPE_EXPR
> -	  || operator_code == COMPONENT_REF
> -	  || operator_code == ARRAY_REF
> -	  || operator_code == NOP_EXPR)
> -	{
> -	  error_at (loc, "%qD must be a non-static member function", decl);
> -	  return false;
> +		     "function only with %<-std=c++23%> or %<-std=gnu++23%>",
> +		     decl);
>   	}
> -
> -      if (DECL_STATIC_FUNCTION_P (decl))
> +      else if (DECL_STATIC_FUNCTION_P (decl))
>   	{
>   	  error_at (loc, "%qD must be either a non-static member "
>   		    "function or a non-member function", decl);
>   	  return false;
>   	}
> -
> -      for (tree arg = argtypes; ; arg = TREE_CHAIN (arg))
> -	{
> -	  if (!arg || arg == void_list_node)
> -	    {
> -	      if (complain)
> -		error_at(loc, "%qD must have an argument of class or "
> -			 "enumerated type", decl);
> -	      return false;
> -	    }
> +      else
> +	for (tree arg = argtypes; ; arg = TREE_CHAIN (arg))
> +	  {
> +	    if (!arg || arg == void_list_node)
> +	      {
> +		if (complain)
> +		  error_at (loc, "%qD must have an argument of class or "
> +			    "enumerated type", decl);
> +		return false;
> +	      }
>         
> -	  tree type = non_reference (TREE_VALUE (arg));
> -	  if (type == error_mark_node)
> -	    return false;
> -	
> -	  /* MAYBE_CLASS_TYPE_P, rather than CLASS_TYPE_P, is used
> -	     because these checks are performed even on template
> -	     functions.  */
> -	  if (MAYBE_CLASS_TYPE_P (type)
> -	      || TREE_CODE (type) == ENUMERAL_TYPE)
> -	    break;
> -	}
> +	    tree type = non_reference (TREE_VALUE (arg));
> +	    if (type == error_mark_node)
> +	      return false;
> +
> +	    /* MAYBE_CLASS_TYPE_P, rather than CLASS_TYPE_P, is used
> +	       because these checks are performed even on template
> +	       functions.  */
> +	    if (MAYBE_CLASS_TYPE_P (type)
> +		|| TREE_CODE (type) == ENUMERAL_TYPE)
> +	      break;
> +	  }
>       }
>   
>     if (operator_code == CALL_EXPR)
> --- gcc/cp/call.cc.jj	2022-11-08 09:54:37.283400616 +0100
> +++ gcc/cp/call.cc	2022-11-10 22:26:38.096936491 +0100
> @@ -6589,12 +6589,36 @@ add_operator_candidates (z_candidate **c
>         if (fns == error_mark_node)
>   	return error_mark_node;
>         if (fns)
> -	add_candidates (BASELINK_FUNCTIONS (fns),
> -			NULL_TREE, arglist, NULL_TREE,
> -			NULL_TREE, false,
> -			BASELINK_BINFO (fns),
> -			BASELINK_ACCESS_BINFO (fns),
> -			flags, candidates, complain);
> +	{
> +	  if (code == ARRAY_REF)
> +	    {
> +	      vec<tree,va_gc> *restlist = make_tree_vector ();
> +	      for (unsigned i = 1; i < nargs; ++i)
> +		vec_safe_push (restlist, (*arglist)[i]);
> +	      z_candidate *save_cand = *candidates;
> +	      add_candidates (BASELINK_FUNCTIONS (fns),
> +			      (*arglist)[0], restlist, NULL_TREE,
> +			      NULL_TREE, false,
> +			      BASELINK_BINFO (fns),
> +			      BASELINK_ACCESS_BINFO (fns),
> +			      flags, candidates, complain);
> +	      /* Release the vec if we didn't add a candidate that uses it.  */
> +	      for (z_candidate *c = *candidates; c != save_cand; c = c->next)
> +		if (c->args == restlist)
> +		  {
> +		    restlist = NULL;
> +		    break;
> +		  }
> +	      release_tree_vector (restlist);
> +	    }
> +	  else
> +	    add_candidates (BASELINK_FUNCTIONS (fns),
> +			    NULL_TREE, arglist, NULL_TREE,
> +			    NULL_TREE, false,
> +			    BASELINK_BINFO (fns),
> +			    BASELINK_ACCESS_BINFO (fns),
> +			    flags, candidates, complain);
> +	}
>       }
>     /* Per [over.match.oper]3.2, if no operand has a class type, then
>        only non-member functions that have type T1 or reference to
> --- gcc/testsuite/g++.dg/cpp23/subscript9.C.jj	2022-11-10 21:06:58.155974655 +0100
> +++ gcc/testsuite/g++.dg/cpp23/subscript9.C	2022-11-11 08:26:26.502445107 +0100
> @@ -0,0 +1,29 @@
> +// P2589R1
> +// { dg-do run { target c++23 } }
> +
> +extern "C" void abort ();
> +
> +struct S
> +{
> +  S () {};
> +  static int &operator[] () { return a[0]; }
> +  static int &operator[] (int x) { return a[x]; }
> +  static int &operator[] (int x, long y) { return a[x + y * 8]; }
> +  static int a[64];
> +};
> +int S::a[64];
> +
> +int
> +main ()
> +{
> +  S s;
> +  for (int i = 0; i < 64; i++)
> +    s.a[i] = 64 - i;
> +  if (s[] != 64 || s[3] != 61 || s[4, 5] != 20)
> +    abort ();
> +  s[]++;
> +  s[42]++;
> +  ++s[3, 2];
> +  if (s.a[0] != 65 || s.a[42] != 23 || s.a[19] != 46)
> +    abort ();
> +}
> --- gcc/testsuite/g++.dg/cpp23/feat-cxx2b.C.jj	2022-10-06 08:56:28.785125656 +0200
> +++ gcc/testsuite/g++.dg/cpp23/feat-cxx2b.C	2022-11-10 22:31:49.187698955 +0100
> @@ -556,8 +556,8 @@
>   
>   #ifndef __cpp_multidimensional_subscript
>   #  error "__cpp_multidimensional_subscript"
> -#elif __cpp_multidimensional_subscript != 202110
> -#  error "__cpp_multidimensional_subscript != 202110"
> +#elif __cpp_multidimensional_subscript != 202211
> +#  error "__cpp_multidimensional_subscript != 202211"
>   #endif
>   
>   #ifndef __cpp_named_character_escapes
> --- gcc/testsuite/g++.old-deja/g++.bugs/900210_10.C.jj	2020-01-14 20:02:47.021606269 +0100
> +++ gcc/testsuite/g++.old-deja/g++.bugs/900210_10.C	2022-11-11 01:41:13.840971578 +0100
> @@ -9,7 +9,7 @@
>   // keywords: operator[], static function members
>   
>   struct struct0 {
> -  static int operator[] ();		/* { dg-error "" } */
> +  static int operator[] ();		/* { dg-error "" "" { target c++20_down } } */
>   };
>   
>   int main () { return 0; }
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624]
  2022-11-14 23:29 ` Jason Merrill
@ 2022-11-15 12:28   ` Jakub Jelinek
  2022-11-15 21:49     ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-11-15 12:28 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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.

--- 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624]
  2022-11-15 12:28   ` [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624] Jakub Jelinek
@ 2022-11-15 21:49     ` Jason Merrill
  2022-11-16  8:25       ` [PATCH] c++, v2: " Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-11-15 21:49 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] c++, v2: Fix up calls to static operator() or operator[] [PR107624]
  2022-11-15 21:49     ` Jason Merrill
@ 2022-11-16  8:25       ` Jakub Jelinek
  2022-11-16 13:13         ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2022-11-16  8:25 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Nov 15, 2022 at 04:49:27PM -0500, Jason Merrill wrote:
> > 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?

So like this?

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

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): Likewise.
	* 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 23:10:14.077666314 +0100
@@ -5017,6 +5017,33 @@ build_operator_new_call (tree fnname, ve
    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
       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,
 		  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
 	      /* 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, tr
 	      /* 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
--- 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++, v2: Fix up calls to static operator() or operator[] [PR107624]
  2022-11-16  8:25       ` [PATCH] c++, v2: " Jakub Jelinek
@ 2022-11-16 13:13         ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2022-11-16 13:13 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 11/16/22 03:25, Jakub Jelinek wrote:
> On Tue, Nov 15, 2022 at 04:49:27PM -0500, Jason Merrill wrote:
>>> 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?
> 
> So like this?
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 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): Likewise.
> 	* 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 23:10:14.077666314 +0100
> @@ -5017,6 +5017,33 @@ build_operator_new_call (tree fnname, ve
>      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
>         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,
>   		  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
>   	      /* 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, tr
>   	      /* 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
> --- 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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-16 13:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11  7:40 [PATCH] c++: Implement C++23 P2589R1 - - static operator[] Jakub Jelinek
2022-11-14 23:29 ` Jason Merrill
2022-11-15 12:28   ` [PATCH] c++: Fix up calls to static operator() or operator[] [PR107624] Jakub Jelinek
2022-11-15 21:49     ` Jason Merrill
2022-11-16  8:25       ` [PATCH] c++, v2: " Jakub Jelinek
2022-11-16 13:13         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).