This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Start Fortran support for variable objects.


 > >                         ...(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).

OK, I'll remember that in future.

 > 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.

It comes back to what I said earlier: In reality GDB doesn't support more than
one version of MI (the current one).  Even the recently implemented MI command
-stack-info-frame will work with -i=mi1.  There are small differences in
output format e.g the preamble that GDB prints out is put on the log stream
for mi2 but not mi1, but thats about it.  I would favour just supporting one
level with tests mi-*.exp.  That seems to be hard enough.  Apple seem to be
the only ones who are already using MI, and they have their own version
anyway.

 > 
 > > 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.

OK.

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

Would this work?

  struct type *type;

      ...
      type = get_type (var);
      if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
	j = i + TYPE_LOW_BOUND (TYPE_INDEX_TYPE (var->type));
      else
	j = i;

 > > +      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"?

I'll think this over (and the change above) and then submit another patch.

Nick


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]