Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Nick Roberts <nickrob@snap.net.nz>
Cc: Wu Zhou <woodzltc@cn.ibm.com>, gdb-patches@sources.redhat.com
Subject: Re: PATCH: Start Fortran support for variable objects.
Date: Mon, 04 Jul 2005 03:49:00 -0000	[thread overview]
Message-ID: <20050704034904.GA5802@nevyn.them.org> (raw)
In-Reply-To: <17096.37817.638887.840041@farnswood.snap.net.nz>

On Mon, Jul 04, 2005 at 01:41:13PM +1200, Nick Roberts wrote:
>  > >  > First of all, never reference ->main_type - see above for the right way
>  > >  > to get the low bound.  An even better way (it seems) is to call
>  > >  > get_discrete_bounds.  Take a look at value_subscript for an example.
>  > > 
>  > > Better than TYPE_LOW_BOUND?
>  > 
>  > I suppose.  I don't know which one is preferred; some day, someone
>  > should go through and clean them all up to be consistent.  I'm fine
>  > with either choice.
> 
> To keep things simple I've used TYPE_LOW_BOUND.  I've tested with the examples
> I've posted before and it works.  Presumably there should also be a test case,
> so I'll create one for mi-var-child.exp and mi2-var-child.exp (you still
> haven't approved my patch for mi2-cmd-stack.exp (28 Jun 2005 01:53:52 +1200).

You posted nothing to gdb-patches on June 27th, 28th, or 29th (except
for the first version of this patch).  I vaguely remember seeing a
patch on gdb@ when Mark complained about your introducing regressions.
But if you'd like it approved, please post it to the patches list; I am
methodical about processing gdb-patches mail because it has a clearly
defined request-reply format, and gdb@ discussions tend to wander off
on tangents (like that one did).

BTW, found the patch in the archives - the changelog entry is for the
wrong file.  Also, can we just remove the failing test, instead of
adding new tests to mi2?  We really need to get a coherent story
together on what "is" mi2, but I don't think we need to add tests for
new commands to it.

> 2005-06-30  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* varobj.c (varobj_list_children): Allow non-zero offsets for
> 	languages like Fortran.

Retcode is unused.

Can't we get here with struct types?  In which case this will blow up:

> +      j = i + TYPE_LOW_BOUND (TYPE_INDEX_TYPE (var->type));
> +
>        /* check if child exists, if not create */
> -      name = name_of_child (var, i);
> +      name = name_of_child (var, j);
>        child = child_exists (var, name);
>        if (child == NULL)
> -	child = create_child (var, i, name);
> +	child = create_child (var, j, name);
>  
>        *((*childlist) + i) = child;
>      }

Also, I'm beginning to wonder if you're doing this in the right place. 
Not that it matters a whole lot, but index is 0-based in every other
case, including for structs.  Maybe the children of arr(4) should be
arr.0 == arr(1), arr.1 == arr(2), arr.2 == arr(3), arr.3 == arr(4). 
Then you'd add the lower bound in c_value_of_child.  Does that work?
Do you have an opinion on which is "more right"?

-- 
Daniel Jacobowitz
CodeSourcery, LLC


  reply	other threads:[~2005-07-04  3:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-06-29 21:28 Nick Roberts
2005-06-30  2:53 ` Daniel Jacobowitz
2005-06-30  9:28   ` Nick Roberts
2005-06-30 13:15     ` Daniel Jacobowitz
2005-06-30 22:21       ` Nick Roberts
2005-06-30 22:23         ` Daniel Jacobowitz
2005-06-30 13:18 ` Daniel Jacobowitz
2005-06-30 22:21   ` Nick Roberts
2005-07-01  3:35     ` Wu Zhou
2005-07-01  5:04       ` Nick Roberts
2005-07-01 12:00         ` Wu Zhou
2005-07-03 16:17         ` Daniel Jacobowitz
2005-07-03 23:40           ` Nick Roberts
2005-07-03 23:47             ` Daniel Jacobowitz
2005-07-04  1:42               ` Nick Roberts
2005-07-04  3:49                 ` Daniel Jacobowitz [this message]
2005-07-04  7:35                   ` Nick Roberts
2005-07-05  3:43                   ` Nick Roberts
2006-03-13 14:08                   ` Nick Roberts
2006-03-24 22:58                     ` Daniel Jacobowitz
2006-03-27  1:25                       ` Nick Roberts
2006-03-27  4:04                         ` Daniel Jacobowitz
2006-03-27  4:24                           ` Nick Roberts
2006-03-27 11:32                             ` Daniel Jacobowitz
2005-07-06  8:31           ` Wu Zhou

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=20050704034904.GA5802@nevyn.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=nickrob@snap.net.nz \
    --cc=woodzltc@cn.ibm.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