Re: [PATCH v14 3/4] Add support for lookup, overload resolution and invocation of C++ debug methods

[Siva Chandra <sivachandra at google dot com>]

On Mon, Apr 21, 2014 at 4:56 PM, Doug Evans <dje@google.com> wrote:
> Another issue I have is this bit of code in, e.g., value_x_binop:
>
>   value_user_defined_op (&arg1, argvec + 1, tstr, &static_memfuncp, 2,
>                          &argvec[0], &dm_worker);
>
>   if (argvec[0])
>     {
>       if (static_memfuncp)
>         {
>           argvec[1] = argvec[0];
>           argvec++;
>         }
>       if (noside == EVAL_AVOID_SIDE_EFFECTS)
>         {
>           struct type *return_type;
>
>           return_type
>             = TYPE_TARGET_TYPE (check_typedef (value_type (argvec[0])));
>           return value_zero (return_type, VALUE_LVAL (arg1));
>         }
>       return call_function_by_hand (argvec[0], 2 - static_memfuncp,
>                                     argvec + 1);
>     }
>   if (dm_worker != NULL)
>     {
>       struct cleanup *dm_worker_cleanup = make_cleanup (xfree, dm_worker);
>       struct value *ret_val = invoke_debug_method (dm_worker, arg1, &arg2, 1);
>
>       do_cleanups (dm_worker_cleanup);
>       return ret_val;
>     }
>
> Does this mean we call the debug method (assuming it wins) for the
> case of noside == EVAL_AVOID_SIDE_EFFECTS?

To be frank, I never understood why this option exists at all. If side
effects (like even reading target memory) are not desired, why
evaluate expressions at all? And, fwiw, there is a piece of code in
find_oload_match which looks up the most derived type of an object
which involves reading the vtable much before all this.

> Here and elsewhere there's logic that the debug method code is not
> being included in, and it makes me want to do things differently.

Can you point me to an example different from EVAL_AVOID_SIDE_EFFECTS?

> Fortunately, I think (though I haven't implemented it) it won't be hard.
> Just like we have TYPE_CODE_INTERNAL_FUNCTION we could also have
> TYPE_CODE_EXTERNAL_FUNCTION, and use that to make debug methods less
> special case.
> clone_debug_method_worker could return a struct value that is a
> TYPE_CODE_EXTERNAL_FUNCTION, and then just before the call to
> call_function_by_hand that would happen for normal methods,
> we'd have a check for TYPE_CODE_EXTERNAL_FUNCTION and call
> call_debug_method instead.

I am putting down my notes and questions.

1. I think TYPE_CODE_INTERNAL_FUNCTIONs can also be implemented in
extension languages. So, what makes them internal and different from
TYPE_CODE_EXTERNAL_FUNCTION? I have tried to convince myself that the
name "INTERNAL" exists because of historical reasons and does not mean
anything today. Hence, is TYPE_CODE_EXTERNAL_FUNCTION a good name? Am
I missing something?

2.If debug methods are to be represented by a value (say, of type
TYPE_CODE_DEBUG_METHOD), then a new value should be created for every
debug method invocation. I do not know if this is good or bad, and I
do not know if this happens for internal functions as well. A
fundamental difference (atleast in my mind) is that internal functions
have a name by definition (at least currently) and are invoked by that
name. Debug methods do not have a name by definition that they are
invoked with. In fact, debug methods have different invocation names
based on the context (ie. template arguments).

3. Extending debug methods to functions (if that is what you meant by
making debug methods less special case); I think this would lead to a
lot of confusion between convenience functions, debug methods and
"debug functions". Best would be to support only two mutually
exclusive features "Debug Methods" and "Convenience Functions" with
latter extended to have the ability to replace existing source
functions. This is my personal opinion which I am ready to change if
you prescribe a specific different way.

4. What benefit, apart from lesser number of arguments to
find_oload_match, do you see? I guess this is related to the above
question on what else is the debug methods logic currently missing.

Thanks,
Siva Chandra