[PATCH] Allow calling of user-defined function call operators

[PATCH] Allow calling of user-defined function call operators

[Hannes Domani]

Currently it's not possible to call user-defined function call
operators, at least not without specifying operator() directly:
```
(gdb) l 1
1       struct S {
2         int operator() (int x) { return x + 5; }
3       };
4
5       int main () {
6         S s;
7
8         return s(23);
9       }
(gdb) p s(10)
Invalid data type for function to be called.
(gdb) p s.operator()(10)
$1 = 15
```

This now looks if an user-defined call operator is available when
trying to 'call' a struct value, and calls it instead, making this
possible:
```
(gdb) p s(10)
$1 = 15
```

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
---
 gdb/eval.c                       | 46 +++++++++++++++++++++++++++-----
 gdb/testsuite/gdb.cp/userdef.cc  | 18 +++++++++++++
 gdb/testsuite/gdb.cp/userdef.exp |  4 +++
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 6b752e70635..e737774ca28 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -664,14 +664,34 @@ operation::evaluate_funcall (struct type *expect_type,
 
   value *callee = evaluate_with_coercion (exp, noside);
   struct type *type = callee->type ();
-  if (type->code () == TYPE_CODE_PTR)
-    type = type->target_type ();
-  for (int i = 0; i < args.size (); ++i)
+
+  /* If the callee is a struct, there might be a user-defined function call
+     operator that should be used instead.  */
+  if (overload_resolution
+      && exp->language_defn->la_language == language_cplus
+      && check_typedef (type)->code () == TYPE_CODE_STRUCT)
     {
-      if (i < type->num_fields ())
-	vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
-      else
+      for (int i = 0; i < args.size (); ++i)
 	vals[i] = args[i]->evaluate_with_coercion (exp, noside);
+
+      vals.insert (vals.begin(), value_addr (callee));
+      int static_memfuncp;
+      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
+			   &callee, nullptr, &static_memfuncp, 0, noside);
+      if (static_memfuncp)
+	vals.erase (vals.begin ());
+    }
+  else
+    {
+      if (type->code () == TYPE_CODE_PTR)
+	type = type->target_type ();
+      for (int i = 0; i < args.size (); ++i)
+	{
+	  if (i < type->num_fields ())
+	    vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
+	  else
+	    vals[i] = args[i]->evaluate_with_coercion (exp, noside);
+	}
     }
 
   return evaluate_subexp_do_call (exp, noside, callee, vals,
@@ -702,6 +722,20 @@ var_value_operation::evaluate_funcall (struct type *expect_type,
   value *callee = evaluate_var_value (noside, std::get<0> (m_storage).block,
 				      symp);
 
+  /* If the callee is a struct, there might be a user-defined function call
+     operator that should be used instead.  */
+  if (overload_resolution
+      && exp->language_defn->la_language == language_cplus
+      && check_typedef (callee->type ())->code () == TYPE_CODE_STRUCT)
+    {
+      argvec.insert (argvec.begin(), value_addr (callee));
+      int static_memfuncp;
+      find_overload_match (argvec, "operator()", METHOD, &argvec[0], nullptr,
+			   &callee, nullptr, &static_memfuncp, 0, noside);
+      if (static_memfuncp)
+	argvec.erase (argvec.begin ());
+    }
+
   return evaluate_subexp_do_call (exp, noside, callee, argvec,
 				  nullptr, expect_type);
 }
diff --git a/gdb/testsuite/gdb.cp/userdef.cc b/gdb/testsuite/gdb.cp/userdef.cc
index 774191726f3..48507551079 100644
--- a/gdb/testsuite/gdb.cp/userdef.cc
+++ b/gdb/testsuite/gdb.cp/userdef.cc
@@ -68,6 +68,9 @@ A1 operator++(int);
 A1 operator--(); 
 A1 operator--(int);
 
+int operator()();
+int operator()(int);
+
 };
 
 
@@ -293,6 +296,16 @@ ostream& operator<<(ostream& outs, A1 one)
  return (outs << endl << "x = " << one.x << endl << "y = " << one.y << endl << "-------" << endl); 
 }
 
+int A1::operator()()
+{
+  return x + y;
+}
+
+int A1::operator()(int value)
+{
+  return value * (x + y);
+}
+
 class A2 {
   public:
 A2 operator+();
@@ -404,6 +417,11 @@ int main (void)
  ++three;
  cout << "preinc " << three;
 
+ val = two();
+ cout << "funcall " << val << endl;
+ val = two(10);
+ cout << "funcall 2 " << val << endl;
+
  (*c).z = 1;
 
  return 0;
diff --git a/gdb/testsuite/gdb.cp/userdef.exp b/gdb/testsuite/gdb.cp/userdef.exp
index e96636bef0c..667bded6b83 100644
--- a/gdb/testsuite/gdb.cp/userdef.exp
+++ b/gdb/testsuite/gdb.cp/userdef.exp
@@ -119,6 +119,10 @@ gdb_test "print one += 7" "\\\$\[0-9\]* = {x = 9, y = 10}"
 
 gdb_test "print two = one" "\\\$\[0-9\]* = {x = 9, y = 10}"
 
+gdb_test "print two()" " = 19"
+gdb_test "print two(10)" " = 190"
+gdb_test "print (*&two)(2)" " = 38"
+
 # Check that GDB tolerates whitespace in operator names.
 gdb_test "break A2::operator+" ".*Breakpoint $decimal at.*"
 gdb_test "break A2::operator +" ".*Breakpoint $decimal at.*"
-- 
2.35.1

Re: [PATCH] Allow calling of user-defined function call operators

[Guinevere Larsen]

On 4/21/24 09:49, Hannes Domani wrote:
> Currently it's not possible to call user-defined function call
> operators, at least not without specifying operator() directly:
> ```
> (gdb) l 1
> 1       struct S {
> 2         int operator() (int x) { return x + 5; }
> 3       };
> 4
> 5       int main () {
> 6         S s;
> 7
> 8         return s(23);
> 9       }
> (gdb) p s(10)
> Invalid data type for function to be called.
> (gdb) p s.operator()(10)
> $1 = 15
> ```
>
> This now looks if an user-defined call operator is available when
> trying to 'call' a struct value, and calls it instead, making this
> possible:
> ```
> (gdb) p s(10)
> $1 = 15
> ```
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213

Thanks for this fix, this has been a long time coming.

I have one, possibly big, question.

> ---
>   gdb/eval.c                       | 46 +++++++++++++++++++++++++++-----
>   gdb/testsuite/gdb.cp/userdef.cc  | 18 +++++++++++++
>   gdb/testsuite/gdb.cp/userdef.exp |  4 +++
>   3 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 6b752e70635..e737774ca28 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -664,14 +664,34 @@ operation::evaluate_funcall (struct type *expect_type,
>   
>     value *callee = evaluate_with_coercion (exp, noside);
>     struct type *type = callee->type ();
> -  if (type->code () == TYPE_CODE_PTR)
> -    type = type->target_type ();
> -  for (int i = 0; i < args.size (); ++i)
> +
> +  /* If the callee is a struct, there might be a user-defined function call
> +     operator that should be used instead.  */
> +  if (overload_resolution
> +      && exp->language_defn->la_language == language_cplus
> +      && check_typedef (type)->code () == TYPE_CODE_STRUCT)
>       {
> -      if (i < type->num_fields ())
> -	vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
> -      else
> +      for (int i = 0; i < args.size (); ++i)
>   	vals[i] = args[i]->evaluate_with_coercion (exp, noside);
> +
> +      vals.insert (vals.begin(), value_addr (callee));
> +      int static_memfuncp;
> +      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
> +			   &callee, nullptr, &static_memfuncp, 0, noside);
> +      if (static_memfuncp)
> +	vals.erase (vals.begin ());
> +    }
> +  else
> +    {
> +      if (type->code () == TYPE_CODE_PTR)
> +	type = type->target_type ();
> +      for (int i = 0; i < args.size (); ++i)
> +	{
> +	  if (i < type->num_fields ())
> +	    vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
> +	  else
> +	    vals[i] = args[i]->evaluate_with_coercion (exp, noside);
> +	}

This change had me confused for quite a bit. I couldn't figure out why 
the base operation::evaluate_funcall. The problem seems to be that if 
command used is something less straightforward, like "print (foo+bar) 
(args)", we will use the evaluate_funcall of the operation (in this 
case, add_operation) instead of var_value_operation's. Did I understand 
it correctly?

Assuming I did, I don't think this code duplication is the best way to 
go. Especially seeing as there are some differences in those functions 
already, and if all that it takes to defeat out expression parser is use 
(*&foo) or (a + b) (), we probably already have latent bugs in this 
area. I don't know how to verify it, though, because I really don't 
understand the code differences.

Ideally, I think the best way would be to put all the code in 
var_value_operation::evaluate_funcall, but to do this, you'd need to be 
able to find a way to call the resulting var_value_operation's function 
from all relevant operations, which is probably a lot.

Another option is to just park all the logic in 
operation::evaluate_funcall, so we're always using the correct function. 
This comes with the cost of being very confusing in a couple of months 
to a year.

Does this make sense?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>       }
>   
>     return evaluate_subexp_do_call (exp, noside, callee, vals,
> @@ -702,6 +722,20 @@ var_value_operation::evaluate_funcall (struct type *expect_type,
>     value *callee = evaluate_var_value (noside, std::get<0> (m_storage).block,
>   				      symp);
>   
> +  /* If the callee is a struct, there might be a user-defined function call
> +     operator that should be used instead.  */
> +  if (overload_resolution
> +      && exp->language_defn->la_language == language_cplus
> +      && check_typedef (callee->type ())->code () == TYPE_CODE_STRUCT)
> +    {
> +      argvec.insert (argvec.begin(), value_addr (callee));
> +      int static_memfuncp;
> +      find_overload_match (argvec, "operator()", METHOD, &argvec[0], nullptr,
> +			   &callee, nullptr, &static_memfuncp, 0, noside);
> +      if (static_memfuncp)
> +	argvec.erase (argvec.begin ());
> +    }
> +
>     return evaluate_subexp_do_call (exp, noside, callee, argvec,
>   				  nullptr, expect_type);
>   }
> diff --git a/gdb/testsuite/gdb.cp/userdef.cc b/gdb/testsuite/gdb.cp/userdef.cc
> index 774191726f3..48507551079 100644
> --- a/gdb/testsuite/gdb.cp/userdef.cc
> +++ b/gdb/testsuite/gdb.cp/userdef.cc
> @@ -68,6 +68,9 @@ A1 operator++(int);
>   A1 operator--();
>   A1 operator--(int);
>   
> +int operator()();
> +int operator()(int);
> +
>   };
>   
>   
> @@ -293,6 +296,16 @@ ostream& operator<<(ostream& outs, A1 one)
>    return (outs << endl << "x = " << one.x << endl << "y = " << one.y << endl << "-------" << endl);
>   }
>   
> +int A1::operator()()
> +{
> +  return x + y;
> +}
> +
> +int A1::operator()(int value)
> +{
> +  return value * (x + y);
> +}
> +
>   class A2 {
>     public:
>   A2 operator+();
> @@ -404,6 +417,11 @@ int main (void)
>    ++three;
>    cout << "preinc " << three;
>   
> + val = two();
> + cout << "funcall " << val << endl;
> + val = two(10);
> + cout << "funcall 2 " << val << endl;
> +
>    (*c).z = 1;
>   
>    return 0;
> diff --git a/gdb/testsuite/gdb.cp/userdef.exp b/gdb/testsuite/gdb.cp/userdef.exp
> index e96636bef0c..667bded6b83 100644
> --- a/gdb/testsuite/gdb.cp/userdef.exp
> +++ b/gdb/testsuite/gdb.cp/userdef.exp
> @@ -119,6 +119,10 @@ gdb_test "print one += 7" "\\\$\[0-9\]* = {x = 9, y = 10}"
>   
>   gdb_test "print two = one" "\\\$\[0-9\]* = {x = 9, y = 10}"
>   
> +gdb_test "print two()" " = 19"
> +gdb_test "print two(10)" " = 190"
> +gdb_test "print (*&two)(2)" " = 38"
> +
>   # Check that GDB tolerates whitespace in operator names.
>   gdb_test "break A2::operator+" ".*Breakpoint $decimal at.*"
>   gdb_test "break A2::operator +" ".*Breakpoint $decimal at.*"

Re: [PATCH] Allow calling of user-defined function call operators

[Hannes Domani]

 Am Donnerstag, 25. April 2024 um 20:34:19 MESZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:

> On 4/21/24 09:49, Hannes Domani wrote:
> > Currently it's not possible to call user-defined function call
> > operators, at least not without specifying operator() directly:
> > ```
> > (gdb) l 1
> > 1      struct S {
> > 2        int operator() (int x) { return x + 5; }
> > 3      };
> > 4
> > 5      int main () {
> > 6        S s;
> > 7
> > 8        return s(23);
> > 9      }
> > (gdb) p s(10)
> > Invalid data type for function to be called.
> > (gdb) p s.operator()(10)
> > $1 = 15
> > ```
> >
> > This now looks if an user-defined call operator is available when
> > trying to 'call' a struct value, and calls it instead, making this
> > possible:
> > ```
> > (gdb) p s(10)
> > $1 = 15
> > ```
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
>
> Thanks for this fix, this has been a long time coming.
>
> I have one, possibly big, question.
>
> > ---
> >  gdb/eval.c                      | 46 +++++++++++++++++++++++++++-----
> >  gdb/testsuite/gdb.cp/userdef.cc  | 18 +++++++++++++
> >  gdb/testsuite/gdb.cp/userdef.exp |  4 +++
> >  3 files changed, 62 insertions(+), 6 deletions(-)
> >
> > diff --git a/gdb/eval.c b/gdb/eval.c
> > index 6b752e70635..e737774ca28 100644
> > --- a/gdb/eval.c
> > +++ b/gdb/eval.c
> > @@ -664,14 +664,34 @@ operation::evaluate_funcall (struct type *expect_type,
> >
> >    value *callee = evaluate_with_coercion (exp, noside);
> >    struct type *type = callee->type ();
> > -  if (type->code () == TYPE_CODE_PTR)
> > -    type = type->target_type ();
> > -  for (int i = 0; i < args.size (); ++i)
> > +
> > +  /* If the callee is a struct, there might be a user-defined function call
> > +    operator that should be used instead.  */
> > +  if (overload_resolution
> > +      && exp->language_defn->la_language == language_cplus
> > +      && check_typedef (type)->code () == TYPE_CODE_STRUCT)
> >      {
> > -      if (i < type->num_fields ())
> > -    vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
> > -      else
> > +      for (int i = 0; i < args.size (); ++i)
> >      vals[i] = args[i]->evaluate_with_coercion (exp, noside);
> > +
> > +      vals.insert (vals.begin(), value_addr (callee));
> > +      int static_memfuncp;
> > +      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
> > +              &callee, nullptr, &static_memfuncp, 0, noside);
> > +      if (static_memfuncp)
> > +    vals.erase (vals.begin ());
> > +    }
> > +  else
> > +    {
> > +      if (type->code () == TYPE_CODE_PTR)
> > +    type = type->target_type ();
> > +      for (int i = 0; i < args.size (); ++i)
> > +    {
> > +      if (i < type->num_fields ())
> > +        vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
> > +      else
> > +        vals[i] = args[i]->evaluate_with_coercion (exp, noside);
> > +    }
>
> This change had me confused for quite a bit. I couldn't figure out why
> the base operation::evaluate_funcall. The problem seems to be that if
> command used is something less straightforward, like "print (foo+bar)
> (args)", we will use the evaluate_funcall of the operation (in this
> case, add_operation) instead of var_value_operation's. Did I understand
> it correctly?
>
> Assuming I did, I don't think this code duplication is the best way to
> go. Especially seeing as there are some differences in those functions
> already, and if all that it takes to defeat out expression parser is use
> (*&foo) or (a + b) (), we probably already have latent bugs in this
> area. I don't know how to verify it, though, because I really don't
> understand the code differences.
>
> Ideally, I think the best way would be to put all the code in
> var_value_operation::evaluate_funcall, but to do this, you'd need to be
> able to find a way to call the resulting var_value_operation's function
> from all relevant operations, which is probably a lot.
>
> Another option is to just park all the logic in
> operation::evaluate_funcall, so we're always using the correct function.
> This comes with the cost of being very confusing in a couple of months
> to a year.
>
> Does this make sense?

Yes, it does.
And it turns out my patch already didn't work if you e.g. try to use the
operator() on struct members.

Not sure why I didn't see this before, but all evaluate_funcall variations
call evaluate_subexp_do_call at the end, so I just moved the logic there
in this v2: https://sourceware.org/pipermail/gdb-patches/2024-April/208663.html


Hannes