[PATCH] gdb: user variables with components of dynamic type

[Joel Brobecker]

Hi Andrew,

> So your third choice was the winner, the array has dynamic type and
> includes a computed data location.

Thanks for providing the additional details.

> The problem in terms of implementation is that really everything is
> either a real inline value, or a pointer.  All the other words are
> just language sugar on top of these two choices.

Agreed.

> When we look at C++ references (basically pointers + automatic
> dereferencing),

Thanks for sharing the experiment. This was actually a good refresher
for me.

> So we get the behaviour we might expect, the pointer value underlying
> the reference is preserved, but the value pointed too is not.

Yeah. What I took away from this experiment is that I think
the syntax in printing the value gives some clues as to why
some of the underlying value is not captured by the value history
mechanism. Whether this is enough or not will likely depend on
the level of the user, but I tend to think that a reasonably sharp
user seeing @<some-address>: can infer that the value printed
from it is the current value at the address shown. So good enough
for me.

> > It's possible that the compromise you suggest (treat dynamic components
> > the same as pointers) might be the most reasonable way out, but I think
> > it'll invite confusion on the users' side, and probably bug reports.
> > At the very least, I think we should warn users when we do this, so
> > as to be sure to set expectations right, on the spot.
> 
> Adding a warning would be reasonably simple, we can start with (in
> value.c:set_internalvar):
> 
>   if (is_dynamic_type (value_type (new_data.value)))
>     warning ("some warning text here...");
> 
> There's two problems, the first is easy enough to solve:  if the top
> level value being captured is dynamic, then we do capture the _actual_
> value, it's only when a sub-component is dynamic that we have
> problems.  The above check will trigger if only the top-level value is
> dynamic, so it warns in too many places.
> 
> As a concrete example, given this Fortran type:
> 
>   type :: some_type
>      integer, allocatable :: array_one (:,:)
>      integer :: a_field
>      integer, allocatable :: array_two (:,:)
>   end type some_type
> 
>   type(some_type) :: some_var
> 
> Then in GDB:
> 
>   (gdb) set $foo = some_var
> 
> We capture the contents of the some_type struct, including the
> pointers to the dynamic objects array_one and array_two.  But if
> instead we do:
> 
>   (gdb) set $bar = some_var%array_one
> 
> Now we capture the full contents of array_one, there's no further
> dynamic type resolution required.  Changing 'some_var%array_one' will
> not change the value of $bar, but the change would be see in $foo.
> 
> The harder problem is, what warning do we print??  I initially went
> with:
> 
>   components of dynamically typed values are not currently captured within internal variables
> 
> despite being a bit long, it's not immediately clear if a user will
> know what 'dynamically typed values' means?  Maybe we end up needing a
> language specific warning, so for Fortran:
> 
>   the values of allocatable fields are not currently captured within internal variables
> 
> thoughts or suggestions are welcome...

I admit I'm having minor headaches trying to wrap my head around
the various scenarios and how we would want to deal with the situation.

For instance, do we auto-dereference [1] the same way for all languages?
For Ada, for instance, I think we always auto-derefrence, regardless
if the data we're printing is top-level or whether it's part of
an aggregate. It sounds like the same is True for Fortran as well.
But it does look from your experiments that C++ only dereferences
in some of the cases (e.g. when printing a struct with a field being
a reference, the field's value is the address, without the actual
underlying value). This question would have an impact on the warning,
since we would warn only when there is auto-dereferencing.

For the timing of the warning, at least, it seems clear to me that
it would be printed when printing the value. Do we want to print it
when accessing the value too? It would seem logical, I think, but
are there scenarios where this could be counter-productive?

There's also the question of avoiding to get the warning printed
in the middle of the value itself. This might be a bit minor, but
it's something I've always found to be distracting and hard to read,
when a warning is lost with some other data being printed.

> > Have you looked at how we handle components which are references?
> > I wonder how well we handle those...
> 
> As above we treat them as pointers, but guard against possible
> confusion by displaying them as pointers.
> 
> I would not like to change Fortran from displaying dynamnic types as
> their actual value (and instead just display a pointer) as that seems
> like a really bad change just to work around a limitation with
> internal variables.

100% agreed.

> What I think is super interesting is how this all interacts with
> pretty-printers.  So, if I start with this test program:

Interesting indeed.

> I wonder if this problem should just be solved (at least in the
> short/medium term) by improving the documentation for internal
> variables?

For me, if I had to summarize my current thinking thanks to our
discussions so far, I would say:

  (a) The case of fields whose type is dynamic is really very
      similar to the case of references.

  (b) Since we handle references the same as pointers, I think
      it's fine as a first instance to fix our immediate issue,
      which is the internal error, by providing the exact same
      kind of behavior for those dynamic fields as we provide
      for references.

  (c) Finding a way to make this clearer for users would be a nice
      enhancement, but this would be a general enhancement, not
      something required in the context of your patch.

So you and I are converging towards the same solution in the code,
and I agree that a documentation update might be useful as well.

Going back to your original patch, I would need more research
than what I have time for to determine whether I'd fix it
the same way you did at the location you did. After much staring
with the entire function's implementation as a context, the patch
does make sense to me, especially if I ignore a bit the asserts
for a minute.  Hopefully others with a more complete knowledge of
the area of value-saving for the value history can chime in.

I wish I could be more help! :-/

-- 
Joel

[1] I know your situation has to do with dynamic types rather than
references, but I think both cases are in the same boat and could
be treated the same.