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

[Hannes Domani <ssbssa@yahoo.de>]

 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