Re: [PATCH v2][PR gdb/19893] Fix handling of synthetic C++ references

[Pedro Alves <palves at redhat dot com>]

On 05/24/2016 03:07 PM, Martin Galvan wrote:
> Thanks for the answer!
> 
> On Tue, May 24, 2016 at 7:46 AM, Pedro Alves <palves@redhat.com> wrote:
>> ... I still don't know what to think of this -- I simply don't understand it whether
>> you're doing this because it makes sense, or because doing otherwise would be hard
>> to do?
> 
> From a consistency point of view, it's probably not the right thing.
> All of the synthetic pointer cases I've tested always show "<synthetic
> pointer>" instead of "@address".

But normal pointers don't print @address either, only references do.

Not printing "@address" with "set print object off" seems like
hiding information from the user, information that we could show.
We always print it for non-synthetic references, AFAICS.

> Or at least I don't know
> how to do it off the top of my head. I'd have to make value_addr not
> return a not_lval when passing it a synthetic ref, which I'm not sure
> it's right either.

Your comment in the patch, in generic_val_print_ref, reads:

+	 if options->objectprint is true, c_value_print will call value_addr
+	 on the reference, which coerces synthetic references and returns a
+	 'not_lval'.  */

So if that works, I don't understand -- wouldn't calling value_addr
or coerce_ref in generic_val_print_ref if you have a synthetic
reference, or any reference even, be what you'd want?

> 
> I *could*, however, manually call
> value->location.computed.funcs->check_synthetic_pointer in
> generic_val_print_ref instead of using value_bits_synthetic_pointer,
> thus avoiding the check for lval_computed. But that's a bit ugly IMHO.

I don't understand this one.  Only lval_computed values have a 
"location.computed.funcs" to call.

> 
>> - Can you show an example output?  (set print object on/off, etc. whatever might be
>>   handy to clearly explain that that is about).
>>   Pictures are really worth a thousand words.  :-)

So is the problem that this bit:

   if (options->addressprint)
     {
      CORE_ADDR addr
	= extract_typed_address (valaddr + embedded_offset, type);

doesn't work / doesn't make sense with synthetic pointers?

Should we be calling value_addr instead?

Or are we perhaps missing a lval_funcs method?  (Ideally, all
value properties/methods would go through a vtable like
lval_funcs; think "making struct value a proper C++ class" going
forward.)

> Here, 0x601038 is the address of the structure 'ref' is referencing.
> This is consistent with the output for non-synthetic references, where
> the referenced value's address is shown.
> 
>> - Is this covered by any testcase?  I looked for "object" in the whole patch and
>>   didn't seem to find it.
> 
> Not that I know of. Should I add a test for this to implref-struct?

I don't know where, but I think this should indeed be covered by
tests somewhere.

Thanks,
Pedro Alves