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

[PATCH v2] 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
```

The change in operation::evaluate_funcall is to make sure the type
fields are only used for function types, only they use them as the
argument types.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
---
v2:
- Move the logic into evaluate_subexp_do_call, to avoid duplication in
  every evaluate_funcall of each operation subclass.
  This makes it now work for some cases it didn't in v1, like if it's
  called on a class member (`print c.m(5)` in the new test).
- Added tests for other (struct member) operations.
---
 gdb/eval.c                       | 29 ++++++++++++++++++++++++++---
 gdb/testsuite/gdb.cp/userdef.cc  | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.cp/userdef.exp |  7 +++++++
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 6b752e70635..8d5c354f480 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -588,14 +588,35 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
 {
   if (callee == NULL)
     error (_("Cannot evaluate function -- may be inlined"));
+
+  type *ftype = callee->type ();
+
+  /* If the callee is a struct, there might be a user-defined function call
+     operator that should be used instead.  */
+  std::vector<value *> vals;
+  if (overload_resolution
+      && exp->language_defn->la_language == language_cplus
+      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
+    {
+      vals.resize (argvec.size () + 1);
+
+      vals[0] = value_addr (callee);
+      for (int i = 0; i < argvec.size (); ++i)
+	vals[i + 1] = argvec[i];
+
+      int static_memfuncp;
+      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
+			   &callee, nullptr, &static_memfuncp, 0, noside);
+      if (!static_memfuncp)
+	argvec = vals;
+    }
+
   if (noside == EVAL_AVOID_SIDE_EFFECTS)
     {
       /* If the return type doesn't look like a function type,
 	 call an error.  This can happen if somebody tries to turn
 	 a variable into a function call.  */
 
-      type *ftype = callee->type ();
-
       if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
 	{
 	  /* We don't know anything about what the internal
@@ -666,9 +687,11 @@ operation::evaluate_funcall (struct type *expect_type,
   struct type *type = callee->type ();
   if (type->code () == TYPE_CODE_PTR)
     type = type->target_type ();
+  bool type_has_arguments
+    = type->code () == TYPE_CODE_FUNC || type->code () == TYPE_CODE_METHOD;
   for (int i = 0; i < args.size (); ++i)
     {
-      if (i < type->num_fields ())
+      if (type_has_arguments && 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);
diff --git a/gdb/testsuite/gdb.cp/userdef.cc b/gdb/testsuite/gdb.cp/userdef.cc
index 774191726f3..7e045e46b3b 100644
--- a/gdb/testsuite/gdb.cp/userdef.cc
+++ b/gdb/testsuite/gdb.cp/userdef.cc
@@ -307,8 +307,21 @@ class Member
 {
 public:
   int z;
+
+  int operator() ();
+  int operator() (int);
 };
 
+int Member::operator() ()
+{
+  return z;
+}
+
+int Member::operator() (int value)
+{
+  return value * z;
+}
+
 bool operator== (const Member &m1, const Member &m2)
 {
   return m1.z == m2.z;
@@ -335,9 +348,11 @@ int main (void)
  Container c;
  Member mem1, mem2;
  int val;
+ Member Container::* mptr = &Container::m;
  
  mem1.z = 5;
  mem2.z = 7;
+ c.m.z = 8;
 
  marker1(); // marker1-returns-here
  cout << one; // marker1-returns-here
@@ -404,6 +419,13 @@ int main (void)
  ++three;
  cout << "preinc " << three;
 
+ val = mem1 ();
+ cout << "funcall " << val << endl;
+ val = mem1 (10);
+ cout << "funcall 2 " << val << endl;
+ val = (c.*mptr) (11);
+ cout << "funcall 3 " << 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..ce2781977e7 100644
--- a/gdb/testsuite/gdb.cp/userdef.exp
+++ b/gdb/testsuite/gdb.cp/userdef.exp
@@ -132,4 +132,11 @@ gdb_test "ptype &*c" "type = (struct|class) Member {(\[\r\n \]+public:)?\[\r\n \
 gdb_test "print operator== (mem1, mem2)" " = false"
 gdb_test "print operator== (mem1, mem1)" " = true"
 
+gdb_test "print mem1()" " = 5"
+gdb_test "print mem1(10)" " = 50"
+gdb_test "print (*&mem1)(2)" " = 10"
+gdb_test "print (c.*mptr)(3)" " = 24"
+gdb_test "print (&c)->m(4)" " = 32"
+gdb_test "print c.m(5)" " = 40"
+
 gdb_exit
-- 
2.35.1

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

[Guinevere Larsen]

On 4/27/24 13:36, 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
> ```
>
> The change in operation::evaluate_funcall is to make sure the type
> fields are only used for function types, only they use them as the
> argument types.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
> ---
> v2:
> - Move the logic into evaluate_subexp_do_call, to avoid duplication in
>    every evaluate_funcall of each operation subclass.
>    This makes it now work for some cases it didn't in v1, like if it's
>    called on a class member (`print c.m(5)` in the new test).
> - Added tests for other (struct member) operations.
> ---
>   gdb/eval.c                       | 29 ++++++++++++++++++++++++++---
>   gdb/testsuite/gdb.cp/userdef.cc  | 22 ++++++++++++++++++++++
>   gdb/testsuite/gdb.cp/userdef.exp |  7 +++++++
>   3 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 6b752e70635..8d5c354f480 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -588,14 +588,35 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
>   {
>     if (callee == NULL)
>       error (_("Cannot evaluate function -- may be inlined"));
> +
> +  type *ftype = callee->type ();
> +
> +  /* If the callee is a struct, there might be a user-defined function call
> +     operator that should be used instead.  */
> +  std::vector<value *> vals;
> +  if (overload_resolution
> +      && exp->language_defn->la_language == language_cplus
> +      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
> +    {
> +      vals.resize (argvec.size () + 1);
> +
> +      vals[0] = value_addr (callee);
> +      for (int i = 0; i < argvec.size (); ++i)
> +	vals[i + 1] = argvec[i];
> +
> +      int static_memfuncp;
> +      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
> +			   &callee, nullptr, &static_memfuncp, 0, noside);
> +      if (!static_memfuncp)
> +	argvec = vals;

I don't really understand this change. From what I can see in this 
patch, you're just shifting all values in argvec one to the right, to 
add the callee address. Wouldn't this necessitate a change in the logic 
for the rest of the function?

> +    }
> +
>     if (noside == EVAL_AVOID_SIDE_EFFECTS)
>       {
>         /* If the return type doesn't look like a function type,
>   	 call an error.  This can happen if somebody tries to turn
>   	 a variable into a function call.  */
>   
> -      type *ftype = callee->type ();
> -
>         if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
>   	{
>   	  /* We don't know anything about what the internal
> @@ -666,9 +687,11 @@ operation::evaluate_funcall (struct type *expect_type,
>     struct type *type = callee->type ();
>     if (type->code () == TYPE_CODE_PTR)
>       type = type->target_type ();
> +  bool type_has_arguments
> +    = type->code () == TYPE_CODE_FUNC || type->code () == TYPE_CODE_METHOD;
>     for (int i = 0; i < args.size (); ++i)
>       {
> -      if (i < type->num_fields ())
> +      if (type_has_arguments && i < type->num_fields ())
This change also looks to be unrelated to this patch?

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>   	vals[i] = args[i]->evaluate (type->field (i).type (), exp, noside);
>         else
>   	vals[i] = args[i]->evaluate_with_coercion (exp, noside);
> diff --git a/gdb/testsuite/gdb.cp/userdef.cc b/gdb/testsuite/gdb.cp/userdef.cc
> index 774191726f3..7e045e46b3b 100644
> --- a/gdb/testsuite/gdb.cp/userdef.cc
> +++ b/gdb/testsuite/gdb.cp/userdef.cc
> @@ -307,8 +307,21 @@ class Member
>   {
>   public:
>     int z;
> +
> +  int operator() ();
> +  int operator() (int);
>   };
>   
> +int Member::operator() ()
> +{
> +  return z;
> +}
> +
> +int Member::operator() (int value)
> +{
> +  return value * z;
> +}
> +
>   bool operator== (const Member &m1, const Member &m2)
>   {
>     return m1.z == m2.z;
> @@ -335,9 +348,11 @@ int main (void)
>    Container c;
>    Member mem1, mem2;
>    int val;
> + Member Container::* mptr = &Container::m;
>    
>    mem1.z = 5;
>    mem2.z = 7;
> + c.m.z = 8;
>   
>    marker1(); // marker1-returns-here
>    cout << one; // marker1-returns-here
> @@ -404,6 +419,13 @@ int main (void)
>    ++three;
>    cout << "preinc " << three;
>   
> + val = mem1 ();
> + cout << "funcall " << val << endl;
> + val = mem1 (10);
> + cout << "funcall 2 " << val << endl;
> + val = (c.*mptr) (11);
> + cout << "funcall 3 " << 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..ce2781977e7 100644
> --- a/gdb/testsuite/gdb.cp/userdef.exp
> +++ b/gdb/testsuite/gdb.cp/userdef.exp
> @@ -132,4 +132,11 @@ gdb_test "ptype &*c" "type = (struct|class) Member {(\[\r\n \]+public:)?\[\r\n \
>   gdb_test "print operator== (mem1, mem2)" " = false"
>   gdb_test "print operator== (mem1, mem1)" " = true"
>   
> +gdb_test "print mem1()" " = 5"
> +gdb_test "print mem1(10)" " = 50"
> +gdb_test "print (*&mem1)(2)" " = 10"
> +gdb_test "print (c.*mptr)(3)" " = 24"
> +gdb_test "print (&c)->m(4)" " = 32"
> +gdb_test "print c.m(5)" " = 40"
> +
>   gdb_exit

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

[Hannes Domani]

 Am Freitag, 3. Mai 2024 um 20:29:47 MESZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:

> On 4/27/24 13:36, 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
> > ```
> >
> > The change in operation::evaluate_funcall is to make sure the type
> > fields are only used for function types, only they use them as the
> > argument types.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
> > ---
> > v2:
> > - Move the logic into evaluate_subexp_do_call, to avoid duplication in
> >    every evaluate_funcall of each operation subclass.
> >    This makes it now work for some cases it didn't in v1, like if it's
> >    called on a class member (`print c.m(5)` in the new test).
> > - Added tests for other (struct member) operations.
> > ---
> >  gdb/eval.c                      | 29 ++++++++++++++++++++++++++---
> >  gdb/testsuite/gdb.cp/userdef.cc  | 22 ++++++++++++++++++++++
> >  gdb/testsuite/gdb.cp/userdef.exp |  7 +++++++
> >  3 files changed, 55 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/eval.c b/gdb/eval.c
> > index 6b752e70635..8d5c354f480 100644
> > --- a/gdb/eval.c
> > +++ b/gdb/eval.c
> > @@ -588,14 +588,35 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
> >  {
> >    if (callee == NULL)
> >      error (_("Cannot evaluate function -- may be inlined"));
> > +
> > +  type *ftype = callee->type ();
> > +
> > +  /* If the callee is a struct, there might be a user-defined function call
> > +    operator that should be used instead.  */
> > +  std::vector<value *> vals;
> > +  if (overload_resolution
> > +      && exp->language_defn->la_language == language_cplus
> > +      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
> > +    {
> > +      vals.resize (argvec.size () + 1);
> > +
> > +      vals[0] = value_addr (callee);
> > +      for (int i = 0; i < argvec.size (); ++i)
> > +    vals[i + 1] = argvec[i];
> > +
> > +      int static_memfuncp;
> > +      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
> > +              &callee, nullptr, &static_memfuncp, 0, noside);
> > +      if (!static_memfuncp)
> > +    argvec = vals;
>
> I don't really understand this change. From what I can see in this
> patch, you're just shifting all values in argvec one to the right, to
> add the callee address. Wouldn't this necessitate a change in the logic
> for the rest of the function?

Yes, this adds the 'this' pointer as the first argument for operator().
I'm not sure why this would change the logic for the rest of the function.


> > +    }
> > +
> >    if (noside == EVAL_AVOID_SIDE_EFFECTS)
> >      {
> >        /* If the return type doesn't look like a function type,
> >      call an error.  This can happen if somebody tries to turn
> >      a variable into a function call.  */
> >
> > -      type *ftype = callee->type ();
> > -
> >        if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
> >      {
> >        /* We don't know anything about what the internal
> > @@ -666,9 +687,11 @@ operation::evaluate_funcall (struct type *expect_type,
> >    struct type *type = callee->type ();
> >    if (type->code () == TYPE_CODE_PTR)
> >      type = type->target_type ();
> > +  bool type_has_arguments
> > +    = type->code () == TYPE_CODE_FUNC || type->code () == TYPE_CODE_METHOD;
> >    for (int i = 0; i < args.size (); ++i)
> >      {
> > -      if (i < type->num_fields ())
> > +      if (type_has_arguments && i < type->num_fields ())
> This change also looks to be unrelated to this patch?

It's what I described here:

> The change in operation::evaluate_funcall is to make sure the type
> fields are only used for function types, only they use them as the
> argument types.
 
Before this patch it didn't matter if it used the field types in the
evaluate calls, but since the caller type could now also be a struct,
these would be the struct fields types, not function arguments.


Hannes

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

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> The change in operation::evaluate_funcall is to make sure the type
Hannes> fields are only used for function types, only they use them as the
Hannes> argument types.

IIRC the evaluation operations are all kind of complicated and
hairy... but it seems to me that the type of the chosen overload of
operator() would supply the type here?

Hannes> +  type *ftype = callee->type ();
Hannes> +
Hannes> +  /* If the callee is a struct, there might be a user-defined function call
Hannes> +     operator that should be used instead.  */
Hannes> +  std::vector<value *> vals;
Hannes> +  if (overload_resolution
Hannes> +      && exp->language_defn->la_language == language_cplus
Hannes> +      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
Hannes> +    {

One question to consider is whether this should be done in the
expression node or elsewhere.  Other operator overloads are handled in
the value API instead.

I'm not sure which is better.  It would probably be cleaner to do it in
the expression nodes, like you've done.  Actually the best would
probably be to make a new operation subclass and avoid the need for a
language check.

However, the value API is convenient to use -- for example, this is what
makes operator overloading work in the Python API.

You can see the distinction with this patch by trying to call a
struct-with-operator() object from Python.

Tom

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

[Hannes Domani]

 Am Freitag, 3. Mai 2024 um 22:06:46 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
>
> Hannes> The change in operation::evaluate_funcall is to make sure the type
> Hannes> fields are only used for function types, only they use them as the
> Hannes> argument types.
>
> IIRC the evaluation operations are all kind of complicated and
> hairy... but it seems to me that the type of the chosen overload of
> operator() would supply the type here?

Here the overload of operator() is chosen based on the argument values,
not the other way round.


> Hannes> +  type *ftype = callee->type ();
> Hannes> +
> Hannes> +  /* If the callee is a struct, there might be a user-defined function call
> Hannes> +    operator that should be used instead.  */
> Hannes> +  std::vector<value *> vals;
> Hannes> +  if (overload_resolution
> Hannes> +      && exp->language_defn->la_language == language_cplus
> Hannes> +      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
> Hannes> +    {
>
> One question to consider is whether this should be done in the
> expression node or elsewhere.  Other operator overloads are handled in
> the value API instead.
>
> I'm not sure which is better.  It would probably be cleaner to do it in
> the expression nodes, like you've done.  Actually the best would
> probably be to make a new operation subclass and avoid the need for a
> language check.

I've now moved the logic into evaluate_subexp_do_call so all operation
subclasses would profit from this, so I don't understand how a new
operation subclass would work for this.


> However, the value API is convenient to use -- for example, this is what
> makes operator overloading work in the Python API.
>
> You can see the distinction with this patch by trying to call a
> struct-with-operator() object from Python.

Calling a struct-with-operator() object from Python does not work, because
valpy_call directly calls call_function_by_hand.
It would maybe be possible to also call evaluate_subexp_do_call there.


Hannes

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

[Guinevere Larsen]

On 5/3/24 15:51, Hannes Domani wrote:
>   Am Freitag, 3. Mai 2024 um 20:29:47 MESZ hat Guinevere Larsen <blarsen@redhat.com> Folgendes geschrieben:
>
>> On 4/27/24 13:36, 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
>>> ```
>>>
>>> The change in operation::evaluate_funcall is to make sure the type
>>> fields are only used for function types, only they use them as the
>>> argument types.
>>>
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213
>>> ---
>>> v2:
>>> - Move the logic into evaluate_subexp_do_call, to avoid duplication in
>>>      every evaluate_funcall of each operation subclass.
>>>      This makes it now work for some cases it didn't in v1, like if it's
>>>      called on a class member (`print c.m(5)` in the new test).
>>> - Added tests for other (struct member) operations.
>>> ---
>>>    gdb/eval.c                      | 29 ++++++++++++++++++++++++++---
>>>    gdb/testsuite/gdb.cp/userdef.cc  | 22 ++++++++++++++++++++++
>>>    gdb/testsuite/gdb.cp/userdef.exp |  7 +++++++
>>>    3 files changed, 55 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gdb/eval.c b/gdb/eval.c
>>> index 6b752e70635..8d5c354f480 100644
>>> --- a/gdb/eval.c
>>> +++ b/gdb/eval.c
>>> @@ -588,14 +588,35 @@ evaluate_subexp_do_call (expression *exp, enum noside noside,
>>>    {
>>>      if (callee == NULL)
>>>        error (_("Cannot evaluate function -- may be inlined"));
>>> +
>>> +  type *ftype = callee->type ();
>>> +
>>> +  /* If the callee is a struct, there might be a user-defined function call
>>> +    operator that should be used instead.  */
>>> +  std::vector<value *> vals;
>>> +  if (overload_resolution
>>> +      && exp->language_defn->la_language == language_cplus
>>> +      && check_typedef (ftype)->code () == TYPE_CODE_STRUCT)
>>> +    {
>>> +      vals.resize (argvec.size () + 1);
>>> +
>>> +      vals[0] = value_addr (callee);
>>> +      for (int i = 0; i < argvec.size (); ++i)
>>> +    vals[i + 1] = argvec[i];
>>> +
>>> +      int static_memfuncp;
>>> +      find_overload_match (vals, "operator()", METHOD, &vals[0], nullptr,
>>> +              &callee, nullptr, &static_memfuncp, 0, noside);
>>> +      if (!static_memfuncp)
>>> +    argvec = vals;
>> I don't really understand this change. From what I can see in this
>> patch, you're just shifting all values in argvec one to the right, to
>> add the callee address. Wouldn't this necessitate a change in the logic
>> for the rest of the function?
> Yes, this adds the 'this' pointer as the first argument for operator().
Ah, thanks for explaining! I think a comment would be pretty helpful in 
this situation.
> I'm not sure why this would change the logic for the rest of the function.

My thinking is that, on some situations for the rest of the function, 
there may be one too many arguments. To give a concrete example, I 
thought that if you had foo.bar(), where bar is a regular method, this 
could be adding the "this" argument one too many times. But that would 
mean "callee" is 'foo', and re-reading with fresh eyes, callee is 
supposed to be "bar", right?

>
>
>>> +    }
>>> +
>>>      if (noside == EVAL_AVOID_SIDE_EFFECTS)
>>>        {
>>>          /* If the return type doesn't look like a function type,
>>>        call an error.  This can happen if somebody tries to turn
>>>        a variable into a function call.  */
>>>
>>> -      type *ftype = callee->type ();
>>> -
>>>          if (ftype->code () == TYPE_CODE_INTERNAL_FUNCTION)
>>>        {
>>>          /* We don't know anything about what the internal
>>> @@ -666,9 +687,11 @@ operation::evaluate_funcall (struct type *expect_type,
>>>      struct type *type = callee->type ();
>>>      if (type->code () == TYPE_CODE_PTR)
>>>        type = type->target_type ();
>>> +  bool type_has_arguments
>>> +    = type->code () == TYPE_CODE_FUNC || type->code () == TYPE_CODE_METHOD;
>>>      for (int i = 0; i < args.size (); ++i)
>>>        {
>>> -      if (i < type->num_fields ())
>>> +      if (type_has_arguments && i < type->num_fields ())
>> This change also looks to be unrelated to this patch?
> It's what I described here:
>
>> The change in operation::evaluate_funcall is to make sure the type
>> fields are only used for function types, only they use them as the
>> argument types.
>   
> Before this patch it didn't matter if it used the field types in the
> evaluate calls, but since the caller type could now also be a struct,
> these would be the struct fields types, not function arguments.

Ooohh, I see.

Again, I think a code comment would help a lot. Something above the if, 
saying for example:

     "If type is a struct, num_fields would refer to the number of 
members in the type, not the number of arguments"

or similar.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
>
> Hannes
>

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

[Tom Tromey]

>> IIRC the evaluation operations are all kind of complicated and
>> hairy... but it seems to me that the type of the chosen overload of
>> operator() would supply the type here?

Hannes> Here the overload of operator() is chosen based on the argument values,
Hannes> not the other way round.

Ok, I looked a little more and I see other paths pretty much doing the
same thing.  I don't really understand how this works if, say, a call
like this requires a pointer-adjusting cast to be done --
evaluate_with_coercion won't do this properly.  However, if there's a
bug here, it's probably reproducible some other way already.

>> However, the value API is convenient to use -- for example, this is what
>> makes operator overloading work in the Python API.
>> 
>> You can see the distinction with this patch by trying to call a
>> struct-with-operator() object from Python.

Hannes> Calling a struct-with-operator() object from Python does not work, because
Hannes> valpy_call directly calls call_function_by_hand.
Hannes> It would maybe be possible to also call evaluate_subexp_do_call there.

We'll probably just need a new value API if/when the time comes.

Tom

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

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> The change in operation::evaluate_funcall is to make sure the type
Hannes> fields are only used for function types, only they use them as the
Hannes> argument types.

Hannes> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12213

I think this would be ok with the comment update requested by Guinevere.

thanks,
Tom