Do not consider reference types as dynamic

  Joel suggested in private that if we don't consider reference types as 
dynamic in is_dynamic_type_internal, maybe the corresponding handling of 
reference types in resolve_dynamic_type_internal could be removed too.

I've tried to do this and this still triggers no regression on 
x86_64-linux. It's not surprising actually, since 
resolve_dynamic_type_internal does nothing on a type when 
is_dynamic_type_internal returns false for it.

So here's the updated patch. Thanks Joel for the idea! :-)
  

> >From f3fd1a7e81fad7a0b8e4f483f79b2470a3cf1499 Mon Sep 17 00:00:00 2001
> From: Pierre-Marie de Rodat <derodat@adacore.com>
> Date: Fri, 3 Apr 2015 10:40:52 +0200
> Subject: [PATCH] Do not consider reference types as dynamic
> 
> Even when referenced types are dynamic, the corresponding referencing
> type should not be considered as dynamic: it's only a pointer.  This
> prevents reference type for values not in memory to be resolved.
> 
> gdb/ChangeLog:
> 2015-04-03  Pierre-Marie de Rodat  <derodat@adacore.com>
> 
> 	* gdbtypes.c (is_dynamic_type_internal): Remove special handling
> 	of TYPE_CODE_REF types so that they are not considered as
> 	dynamic depending on the referenced type.
> 	(resolve_dynamic_type_internal): Likewise.
> 
> gdb/testsuite/ChangeLog:
> 2015-04-03  Pierre-Marie de Rodat  <derodat@adacore.com>
> 
> 	* gdb.ada/funcall_ref.exp: New file.
> 	* gdb.ada/funcall_ref/foo.adb: New file.

This is OK, thank you, Pierre-Marie.

Note that the parameter "top_level" is no longer useful after
this patch is applied, which is actually quite a nice side-effect
of this patch. I hum'ed and ah'ed about whether to ask that it be
removed as part of this patch, but in the end, I decided that this patch
was fine without the cleanup, as "top_level" is used a several places,
and I felt that cleaning it up here would dilute a bit the essence of
this patch.  So, I think we can clean it up separately.

> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 217ec70..6fb2e9a 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1752,10 +1752,6 @@ is_dynamic_type_internal (struct type *type, int top_level)

Thank you,
  

Pierre-Marie de Rodat <derodat@adacore.com> writes:

> I've tried to do this and this still triggers no regression on
> x86_64-linux. It's not surprising actually, since
> resolve_dynamic_type_internal does nothing on a type when
> is_dynamic_type_internal returns false for it.

Hi, Pierre,
This patch causes a regression in gdb.cp/vla-cxx.exp unfortunately,

 print vlaref^M
 $2 = (int (&)[variable length]) @0x3fffffffe900: {5, 7, 9}^M
 (gdb) FAIL: gdb.cp/vla-cxx.exp: print vlaref

which is shown by buildbot
https://sourceware.org/ml/gdb-testers/2015-q2/msg00360.html

I am not familiar with gdb type and dynamic type, so can't give anything
useful on how to fix this regression.  Any thoughts?
  

Yao,

On 04/17/2015 12:47 PM, Yao Qi wrote:
> Hi, Pierre,

(My first name is "Pierre-Marie" ;-))

> This patch causes a regression in gdb.cp/vla-cxx.exp unfortunately,
>
>   print vlaref^M
>   $2 = (int (&)[variable length]) @0x3fffffffe900: {5, 7, 9}^M
>   (gdb) FAIL: gdb.cp/vla-cxx.exp: print vlaref
>
> which is shown by buildbot
> https://sourceware.org/ml/gdb-testers/2015-q2/msg00360.html
>
> I am not familiar with gdb type and dynamic type, so can't give anything
> useful on how to fix this regression.  Any thoughts?

Thank you for the heads up! I don't know what happens, so I will 
investigate and keep you updated.
  

On 04/17/2015 01:20 PM, Pierre-Marie de Rodat wrote:
>> This patch causes a regression in gdb.cp/vla-cxx.exp unfortunately,
>>
>>   print vlaref^M
>>   $2 = (int (&)[variable length]) @0x3fffffffe900: {5, 7, 9}^M
>>   (gdb) FAIL: gdb.cp/vla-cxx.exp: print vlaref
>>
>> which is shown by buildbot
>> https://sourceware.org/ml/gdb-testers/2015-q2/msg00360.html

This is tough!

For the record, I did not notice this regression because I ran the 
testsuite with GCC's development branch, and this testcase was already 
failing with it. It works with GCC 4.9.2, though.

So my understanding was that types are dynamic when we need runtime 
information to decode them properly (memory layout, range bounds, etc). 
With this definition, reference types are never dynamic since they are 
actually mere pointers (static size, no dynamic semantics).

Printing a reference value is supposed to automatically fetch and print 
the referenced object... and its type. This testcase also tells us that 
it's the "fully resolved type" that must be printed (I mean referenced 
types must be resolved, too).

Here, we have "vlaref": a reference to a VLA (variable-length array), 
hence a reference to a dynamic type (the bounds and thus the size of the 
array are dynamic). Because of the "dynamic type" definition above, the 
referenced type is kept as-is during evaluation. And then at 
type-printing time, we use the dynamic referenced type, hence the 
"variable length" in the output.

If the definition above is correct, then we should enhance 
typeprint.c/c-typeprint.c/etc. to resolve dynamic type as type printing 
goes in order to get the actual array bounds in the output. This means: 
propagating a (possibly null) value in a lot of functions and use it for 
type resolution when possible. I guess this would involve a big patch 
but would also cover the "ptype" case.

I guess another way to "just get things working" would be to revert my 
previous patch and to enhance the dynamic property mechanism 
(dwarf2loc.h) to handle reference values that are not present in memory. 
For instance: extend the property_addr_info structure to hold either the 
address of objects (as today) or the address of the referenced object 
(for reference types).

Thoughts?
  

> If the definition above is correct, then we should enhance
> typeprint.c/c-typeprint.c/etc. to resolve dynamic type as type
> printing goes in order to get the actual array bounds in the output.
> This means: propagating a (possibly null) value in a lot of
> functions and use it for type resolution when possible. I guess this
> would involve a big patch but would also cover the "ptype" case.
> 
> I guess another way to "just get things working" would be to revert
> my previous patch and to enhance the dynamic property mechanism
> (dwarf2loc.h) to handle reference values that are not present in
> memory. For instance: extend the property_addr_info structure to
> hold either the address of objects (as today) or the address of the
> referenced object (for reference types).
> 
> Thoughts?

Thanks for looking into that, Pierre-Marie.

My 2 cents, based on the little amount of experience we've had dealing
with dyanmic types in Ada, and the amount of experience we've had
dealing with the GNAT encodings, I have a feeling that it's best to
maintain reference types as being non-dynamic, and enhance the functions
that print reference objects instead. We should probably compare what
Ada does (ada_val_print_ref) compared to C, for instance, as we handle
those reference values correctly, I believe.
  

On 04/17/2015 05:30 PM, Joel Brobecker wrote:
> My 2 cents, based on the little amount of experience we've had dealing
> with dyanmic types in Ada, and the amount of experience we've had
> dealing with the GNAT encodings, I have a feeling that it's best to
> maintain reference types as being non-dynamic, and enhance the functions
> that print reference objects instead. We should probably compare what
> Ada does (ada_val_print_ref) compared to C, for instance, as we handle
> those reference values correctly, I believe.

Joel and I discussed this live: doing the type printing change is 
probably the best way in terms of "implementation consistency", but it's 
also going to be a lot of work.

So the plan is instead to try to enhance type resolution to handle 
references that are not addressable. In the meantime, I've reverted my 
patches so that the regression disappears. I'll submit another patch...
  

On 04/20/2015 04:27 PM, Pierre-Marie de Rodat wrote:
> Joel and I discussed this live: doing the type printing change is
> probably the best way in terms of "implementation consistency", but it's
> also going to be a lot of work.
>
> So the plan is instead to try to enhance type resolution to handle
> references that are not addressable. In the meantime, I've reverted my
> patches so that the regression disappears. I'll submit another patch...

The patch series Joel submitted last Tuesday[1] solves the original 
issue: the plan I was talking about is actually implemented by these 
patches! So there's no need to go further on this matter.

[1] <https://www.sourceware.org/ml/gdb-patches/2015-04/msg00768.html>