Re: [PATCH v4] gdb: add a symbol* argument to get_return_value

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2022-02-03 06:10, Six, Lancelot wrote:
> [AMD Official Use Only]
> 
>> Hi Lancelot,
>>
>> Every time I look I keep finding nits, sorry :(
> 
> No worry.
> 
>> I have the feeling that value_type is redundant with func_symbol, since the return value type initially comes from the symbol.  So I think we could remove value_type.  In fact, it might have been redundant already, since from `struct value *function`, you should also be able to get the function type.  But there may be edge cases I don't know about.
> 
> To be honest, I did have a similar impression about the function parameter.  Once you have the function symbol, you should be able to get a value containing the function address, so this parameter is probably also redundant.  The reason I did not make this change is because the function's address is obtained with a call to:
> 
>     struct value *function = read_var_value (func_symbol, NULL, frame);
> 
> Moving this call within get_return_value might mean we use a different FRAME argument (get_current_frame ()).  I do not expect it to change anything, but because I am not 100% sure, I did not change that.
> 
>> Do you think this change below (that builds on top of your patch) would work?  Tests gdb.*/*finish*.exp pass here.
> 
> I'll probably try to remove both the value* and type* arguments and see if I have any regression (unless anyone is aware on some edge cases where this can be problematic).
> 
> WDYT?

I wouldn't get rid of the `value *` argument, as you said this is
obtained using read_var_value and the current frame.  So if we don't
pass the `value *`, we would need to pass the frame, there's no point in
doing that change.

I'm just talking about removing the `type *` argument, which in both
callers of get_return_value, is obtained using:

  TYPE_TARGET_TYPE (SYMBOL_TYPE (function));

where `function` is the struct symbol that we also pass to
get_return_value.

Simon