Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Pedro Alves <pedro@codesourcery.com>
Cc: gdb-patches@sourceware.org, Joel Brobecker <brobecker@adacore.com>
Subject: Re: [RFA] valprint.c / *-valprint.c: Don't lose `embedded_offset'
Date: Thu, 07 Oct 2010 18:13:00 -0000	[thread overview]
Message-ID: <m3eic1yiq1.fsf@fleche.redhat.com> (raw)
In-Reply-To: <201010070207.02695.pedro@codesourcery.com> (Pedro Alves's	message of "Thu, 7 Oct 2010 02:07:02 +0100")

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> A bit of explaining on why this is necessary.  In the context of
Pedro> tracepoints, I'm adding support for partial/incomplete objects.
Pedro> That is, say, when printing an array where only a few elements
Pedro> have been collected, print what you can, and print something like
Pedro> "<unavailable>" (like <optimized out>) for what has not been
Pedro> collected.

Looks nice.

Pedro> Trouble is in languages other than C/C++ where the
Pedro> advance-embedded_offset-don't-touch-valaddr-or-address contract
Pedro> isn't compromised in many places.

I think the embedded_offset is very confusing.  Every time I have to fix
a bug in it, I have to re-learn what it really means.  Perhaps next time
I will plan ahead and fix the commentary in value.c to be more clear (to
me at least).

Pedro> I have no way of testing the D or SCM changes, though the former's
Pedro> are quite small, and the latter's, I'm not sure who cares at all.

My belief is that gdb's guile support has bit-rotted -- specifically,
that guile has changed incompatibly.

I will ask.  Perhaps we can remove it.

Pedro> All in all, I think it's just better to be consistent throughout.
Pedro> I know I got mighty confused with some functions taking adjusted
Pedro> `valaddr' and `address', while others taking `embedded_offset'
Pedro> into account.  Maybe some day we will not allow passing around
Pedro> a NULL `val', and thus we will be able to get rid of `valaddr'
Pedro> and `address' parameters throughout, and instead get those from
Pedro> the value directly.  I don't plan to actually do that, but
Pedro> this seems like a step in that direction.

I was going to do this last time I had to add a parameter to the whole
val_print hierarchy, but then blinked.

I think we should just get rid of val_print entirely, and only have
value_print, passing around values.  If that is not efficient enough
(too much copying or something), we can change struct value to make it
efficient.

What do you think of that?

This isn't a high priority for me, either, but I suspect we'll need
another round of changes here to support DW_OP_GNU_implicit_pointer.


I did not read the patch extremely closely.  What I did read looks
reasonable.  Unfortunately the nature of a patch like this is that it is
very hard to know whether some spot was missed :-(

Tom


  reply	other threads:[~2010-10-07 18:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-07  1:07 Pedro Alves
2010-10-07 18:13 ` Tom Tromey [this message]
2010-10-07 19:25   ` Pedro Alves
2010-10-18 12:14     ` Pedro Alves

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=m3eic1yiq1.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.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