Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	"pedro@palves.net" <pedro@palves.net>,
	"philippe.waroquiers@skynet.be" <philippe.waroquiers@skynet.be>,
	"aburgess@redhat.com" <aburgess@redhat.com>,
	 "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>,
	"Schimpe, Christina" <christina.schimpe@intel.com>,
	"lsix@lancelotsix.com" <lsix@lancelotsix.com>,
	"tom@tromey.com" <tom@tromey.com>
Subject: RE: [PATCH v11 2/2] gdb: add shadowed field in '-stack-list-locals/variables' mi commands
Date: Fri, 28 Nov 2025 15:09:31 +0000	[thread overview]
Message-ID: <SA1PR11MB684644BD9BC2A0A0B45B81FBCBDCA@SA1PR11MB6846.namprd11.prod.outlook.com> (raw)
In-Reply-To: <86y0nq8cgs.fsf@gnu.org>

Hi Eli,

Thanks for the feedback.

>> +",line="4"@},@{name="x",type="int",value="3",file="/home/src/name.c",
>> +line="1",shadowed="true"@}]
>> +@end smallexample

Eli>Same comment as in the previous patch regarding multi-line examples.
Abdul>Sure will fix in Patchv12.

Eli>Also, the last line is too long, so please break it into several lines, each one no longer than 72 characters (and preferably even fewer).
Abdul>Sure will break it.

>> +A variable is shadowed when there's another variable with the same 
>> +name which is declared within an inner scope (decision block, method, 
>> +or inner class).  For such cases, its location for the outermost 
>> +scope is followed by @samp{shadowed} attribute.  The location can 
>> +help to locate the instances of shadowed variables.  So, location 
>> +information is only added for shadowed variables.

Eli>Instead, please put here a cross-reference to the other node where this is explained.
Abdul>Sure, will add a cross-reference in Patch v12.

Eli>I still think that all variables should have this annotation.  In the MI output, it will make the job of the front-end which needs to parse this output simpler, since the fields will always be present.

Abdul>Replied on the other patch also. We may continue on this topic in other patch or here as you like. For future reference we already have this discussion [1] in upstream to address similar kind of concern by making it configurable which will address your concern in MI RSP where user will have the option to disable it completely. But to add it to all variables  by default might be some thing which may not be that useful for the reason mentioned in other patch. As this series is ongoing for sometime now so will like to merge it first to fix the purpose it was opened.  But please let me know your thoughts if this will address your concern then once this patch series is merged then we will continue to address it like discussed in [1] or if there is greater concern regarding it then may try to make it part of this patch series already.
[1] https://sourceware.org/pipermail/gdb-patches/2023-November/203961.html

Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org> 
Sent: Friday, November 28, 2025 3:21 PM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: gdb-patches@sourceware.org; pedro@palves.net; philippe.waroquiers@skynet.be; aburgess@redhat.com; Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com>; Schimpe, Christina <christina.schimpe@intel.com>; lsix@lancelotsix.com; tom@tromey.com
Subject: Re: [PATCH v11 2/2] gdb: add shadowed field in '-stack-list-locals/variables' mi commands

> From: Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> Cc: pedro@palves.net,
> 	philippe.waroquiers@skynet.be,
> 	aburgess@redhat.com,
> 	tankut.baris.aktemur@intel.com,
> 	christina.schimpe@intel.com,
> 	lsix@lancelotsix.com,
> 	eliz@gnu.org,
> 	abdul.b.ijaz@intel.com,
> 	tom@tromey.com
> Date: Fri, 28 Nov 2025 13:03:17 +0100
> 
>  gdb/NEWS                                  |   4 +
>  gdb/doc/gdb.texinfo                       |  18 +++
>  gdb/mi/mi-cmd-stack.c                     | 129 ++++++++++++++------
>  gdb/testsuite/gdb.mi/mi-var-shadowing.c   |  48 ++++++++
>  gdb/testsuite/gdb.mi/mi-var-shadowing.exp | 141 
> ++++++++++++++++++++++
>  5 files changed, 305 insertions(+), 35 deletions(-)  create mode 
> 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.c
>  create mode 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.exp

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 616899147c3..034b530a7bf 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -371,6 +371,10 @@ vFile:stat
>     multiple times, and the same mapping was being reused.  In all
>     other cases, this field will have the value 'false'.
>  
> +** GDB now shows "shadowed", "file" and "line" fields in the output
> +   of '-stack-list-locals/variables' mi commands for variables
> +   shadowing case.
> +
>  * Support for stabs debugging format and the a.out/dbx object format is
>    deprecated, and will be removed in GDB 18.
>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 
> 9ea4a2705bf..f761e85ee28 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -35417,6 +35417,24 @@ If the @code{--skip-unavailable} option is 
> specified, local variables  and arguments that are not available are 
> not listed.  Partially  available arguments and local variables are still displayed, however.
>  
> +@smallexample
> +1: int x = 3;
> +2: @{
> +3:       int x = 4;
> +4:       int y = 5;
> +4:       x = 99; // breakpoint-line
> +5: @}
> +(gdb) -stack-list-variables 2
> +^done,variables=[@{name="x",type="int",value="4",file="/home/src/name
> +.c",line="3"@},@{name="y",type="int",value="5",file="/home/src/name.c
> +",line="4"@},@{name="x",type="int",value="3",file="/home/src/name.c",
> +line="1",shadowed="true"@}]
> +@end smallexample

Same comment as in the previous patch regarding multi-line examples.

Also, the last line is too long, so please break it into several lines, each one no longer than 72 characters (and preferably even fewer).

> +A variable is shadowed when there's another variable with the same 
> +name which is declared within an inner scope (decision block, method, 
> +or inner class).  For such cases, its location for the outermost 
> +scope is followed by @samp{shadowed} attribute.  The location can 
> +help to locate the instances of shadowed variables.  So, location 
> +information is only added for shadowed variables.

There's no reason to explain twice what does "shadowed variable" mean.
Instead, please put here a cross-reference to the other node where this is explained.

I still think that all variables should have this annotation.  In the MI output, it will make the job of the front-end which needs to parse this output simpler, since the fields will always be present.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Intel Deutschland GmbH
Registered Address: Dornacher Straße 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht München HRB 186928

      reply	other threads:[~2025-11-28 15:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-28 12:03 [PATCH v11 0/2] add annotation in 'info locals' command for variables shadowing case Abdul Basit Ijaz
2025-11-28 12:03 ` [PATCH v11 1/2] gdb: " Abdul Basit Ijaz
2025-11-28 14:15   ` Eli Zaretskii
2025-11-28 14:37     ` Ijaz, Abdul B
2025-11-28 15:23       ` Eli Zaretskii
2025-11-28 12:03 ` [PATCH v11 2/2] gdb: add shadowed field in '-stack-list-locals/variables' mi commands Abdul Basit Ijaz
2025-11-28 14:21   ` Eli Zaretskii
2025-11-28 15:09     ` Ijaz, Abdul B [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SA1PR11MB684644BD9BC2A0A0B45B81FBCBDCA@SA1PR11MB6846.namprd11.prod.outlook.com \
    --to=abdul.b.ijaz@intel.com \
    --cc=aburgess@redhat.com \
    --cc=christina.schimpe@intel.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=lsix@lancelotsix.com \
    --cc=pedro@palves.net \
    --cc=philippe.waroquiers@skynet.be \
    --cc=tankut.baris.aktemur@intel.com \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox