Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Doug Evans <dje@google.com>
To: Siva Chandra <sivachandra@google.com>
Cc: gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH v14 3/4] Add support for lookup, overload resolution and invocation of C++ debug methods
Date: Mon, 21 Apr 2014 23:56:00 -0000	[thread overview]
Message-ID: <21333.45108.381336.447391@ruffy.mtv.corp.google.com> (raw)
In-Reply-To: <CAGyQ6gyk9exQ9zCW5KcBs+qsyyP_Jj1ZnCsNQY6xny1oWANJMQ@mail.gmail.com>

Siva Chandra writes:
 > Attached is the v14 of this part of the patch series. This version
 > addresses a concern raised by Doug during his v12/v13 review.

Hi.

I've been playing with the patch set a bit.

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?

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.

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.

This would be akin to this code in eval.c:

      if (TYPE_CODE (value_type (argvec[0])) == TYPE_CODE_INTERNAL_FUNCTION)
	return call_internal_function (exp->gdbarch, exp->language_defn,
				       argvec[0], nargs, argvec + 1);

      return call_function_by_hand (argvec[0], nargs, argvec + 1);

[I know the function to do this is currently named invoke_debug_method
but Consistency Is Good makes me want to have:
call_function_by_hand, call_internal_function, and call_debug_method
instead of:
call_function_by_hand, call_internal_function, and invoke_debug_method.]

This might(!) then let us remove the dm_worker arg to find_overload_match.
If a debug method is found *valp is a TYPE_CODE_EXTERNAL_FUNCTION
instead of a TYPE_CODE_FUNC.

Thoughts?


  reply	other threads:[~2014-04-21 23:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15 19:06 Siva Chandra
2014-04-21 23:56 ` Doug Evans [this message]
2014-04-22 20:46   ` Siva Chandra
2014-04-23 20:06     ` Doug Evans
2014-04-23 21:41       ` Siva Chandra
2014-04-24  0:33         ` Doug Evans

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21333.45108.381336.447391@ruffy.mtv.corp.google.com \
    --to=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sivachandra@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox