Re: [RFA] Fix frame argument printing when using auto language mode

Re: [RFA] Fix frame argument printing when using auto language mode

[Xavier Roirand]

Le 2/20/18 à 4:15 PM, Yao Qi a écrit :
> On Mon, Feb 19, 2018 at 12:52 PM, Xavier Roirand <roirand@adacore.com> wrote:
>>
>> The problem is that GDB prints the S parameter in the pck.call_me Ada
>> function using the current language, so the C one, because the program
>> is stopped in a C function, whereas it should use the pck.call_me frame
>> one. This behavior is ok when user manually changes the language but it's
>> not the right one when language is auto.
> 
> Agreed, GDB should use per-frame language instead of global current
> language.  However, instead of switching global variable current_language,
> why don't we pass the per-frame language down to la_val_print?  In
> ada-valprint.c:ada_val_print,
> 
>        ada_val_print_1 (type, embedded_offset, address,
>         stream, recurse, val, options,
>         current_language);
>         ^^^^^^^^^^^^^^^^^
> 
> why don't pass language for ada here?
> 

Hello Yao,

It was my first idea, but it would imply a consequent amount of work 
because when ada_val_print_1 is called, the stack is the following one:

#0  ada_val_print_1
#1  ada_val_print
#2  val_print (got the language of the frame arg)

So if we want to pass the per-frame language down to ada_val_print_1 
then in the generic val_print function, we have to change:

       language->la_val_print (type, embedded_offset, address,
                               stream, recurse, val,
                               &local_opts);
to something like:

       language->la_val_print (type, embedded_offset, address,
                               stream, recurse, val,
                               &local_opts, language);
                                            ^^^^^^^^

Fix me if I'm wrong but it means that <language>_val_print functions for 
all the supported GDB languages have to be modified in order to support 
the language parameter.

Regards.

Re: [RFA] Fix frame argument printing when using auto language mode

[Yao Qi]

On Tue, Feb 20, 2018 at 4:28 PM, Xavier Roirand <roirand@adacore.com> wrote:
>
> It was my first idea, but it would imply a consequent amount of work because
> when ada_val_print_1 is called, the stack is the following one:
>
> #0  ada_val_print_1
> #1  ada_val_print
> #2  val_print (got the language of the frame arg)
>
> So if we want to pass the per-frame language down to ada_val_print_1 then in
> the generic val_print function, we have to change:
>
>       language->la_val_print (type, embedded_offset, address,
>                               stream, recurse, val,
>                               &local_opts);
> to something like:
>
>       language->la_val_print (type, embedded_offset, address,
>                               stream, recurse, val,
>                               &local_opts, language);
>                                            ^^^^^^^^
>

We don't have to do that.  ada_val_print_1 is called in ada_val_print,

      ada_val_print_1 (type, embedded_offset, address,
       stream, recurse, val, options,
       current_language);

and ada_val_print is language->la_val_print in effect, so "language"
is ada_language_defn, and we can rewrite the code above like this,

      ada_val_print_1 (type, embedded_offset, address,
       stream, recurse, val, options,
       &ada_language_defn);

This change may break existing behaviour.  Printing for ada (or other
languages) shouldn't depend on current_language, IMO.

> Fix me if I'm wrong but it means that <language>_val_print functions for all
> the supported GDB languages have to be modified in order to support the
> language parameter.
>

-- 
Yao (齐尧)

Re: [RFA] Fix frame argument printing when using auto language mode

[Xavier Roirand]

Hello,

This is a doable fix, I don't see any issue with it and hope that it 
will not add any side effect. I'll propose an RFA soon. Thx

Regards

Le 2/20/18 à 5:48 PM, Yao Qi a écrit :
> On Tue, Feb 20, 2018 at 4:28 PM, Xavier Roirand <roirand@adacore.com> wrote:
>>
>> It was my first idea, but it would imply a consequent amount of work because
>> when ada_val_print_1 is called, the stack is the following one:
>>
>> #0  ada_val_print_1
>> #1  ada_val_print
>> #2  val_print (got the language of the frame arg)
>>
>> So if we want to pass the per-frame language down to ada_val_print_1 then in
>> the generic val_print function, we have to change:
>>
>>        language->la_val_print (type, embedded_offset, address,
>>                                stream, recurse, val,
>>                                &local_opts);
>> to something like:
>>
>>        language->la_val_print (type, embedded_offset, address,
>>                                stream, recurse, val,
>>                                &local_opts, language);
>>                                             ^^^^^^^^
>>
> 
> We don't have to do that.  ada_val_print_1 is called in ada_val_print,
> 
>        ada_val_print_1 (type, embedded_offset, address,
>         stream, recurse, val, options,
>         current_language);
> 
> and ada_val_print is language->la_val_print in effect, so "language"
> is ada_language_defn, and we can rewrite the code above like this,
> 
>        ada_val_print_1 (type, embedded_offset, address,
>         stream, recurse, val, options,
>         &ada_language_defn);
> 
> This change may break existing behaviour.  Printing for ada (or other
> languages) shouldn't depend on current_language, IMO.
> 
>> Fix me if I'm wrong but it means that <language>_val_print functions for all
>> the supported GDB languages have to be modified in order to support the
>> language parameter.
>>
> 

Re: [RFA] Fix frame argument printing when using auto language mode

[Joel Brobecker]

Hi Yao,

> We don't have to do that.  ada_val_print_1 is called in ada_val_print,
> 
>       ada_val_print_1 (type, embedded_offset, address,
>        stream, recurse, val, options,
>        current_language);
> 
> and ada_val_print is language->la_val_print in effect, so "language"
> is ada_language_defn, and we can rewrite the code above like this,
> 
>       ada_val_print_1 (type, embedded_offset, address,
>        stream, recurse, val, options,
>        &ada_language_defn);
> 
> This change may break existing behaviour.  Printing for ada (or other
> languages) shouldn't depend on current_language, IMO.

Thanks for the extra pair of eyes, and the pertinent suggestions!
FWIW, I agree with you, your fix is much better.

-- 
Joel