[PATCH 35/358] Fix -Wsahdow warnings

[PATCH 35/358] Fix -Wsahdow warnings

[Andrey Smirnov]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Fix -Wshadow warnings --]
[-- Type: text/x-diff, Size: 3501 bytes --]

From b831dfc1178c4501532af794a6a841886ee10b06 Mon Sep 17 00:00:00 2001
From: Andrey Smirnov <andrew.smirnov@gmail.com>
Date: Tue, 22 Nov 2011 17:59:24 +0700
Subject: [PATCH 34/39] Fix -Wshadow warnings.

* breakpoint.c (update_static_tracepoint): Fix -Wshadow
warnings.
---
 gdb/ChangeLog    |    5 +++++
 gdb/breakpoint.c |   30 +++++++++++++++---------------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 77bff37..a99830f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2011-11-22  Andrey Smirnov <andrew.smirnov@gmail.com>
 
+	* breakpoint.c (update_static_tracepoint): Fix -Wshadow
+	warnings.
+
+2011-11-22  Andrey Smirnov <andrew.smirnov@gmail.com>
+
 	* breakpoint.c (update_global_location_list): Fix -Wshadow
 	warnings.
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index cba4d3e..9a7f132 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11640,26 +11640,26 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 
       if (!VEC_empty(static_tracepoint_marker_p, markers))
 	{
-	  struct symtab_and_line sal;
+	  struct symtab_and_line las;
 	  struct symbol *sym;
-	  struct static_tracepoint_marker *marker;
+	  struct static_tracepoint_marker *tpmarker;
 	  struct ui_out *uiout = current_uiout;
 
-	  marker = VEC_index (static_tracepoint_marker_p, markers, 0);
+	  tpmarker = VEC_index (static_tracepoint_marker_p, markers, 0);
 
 	  xfree (tp->static_trace_marker_id);
-	  tp->static_trace_marker_id = xstrdup (marker->str_id);
+	  tp->static_trace_marker_id = xstrdup (tpmarker->str_id);
 
 	  warning (_("marker for static tracepoint %d (%s) not "
 		     "found at previous line number"),
 		   b->number, tp->static_trace_marker_id);
 
-	  init_sal (&sal);
+	  init_sal (&las);
 
-	  sal.pc = marker->address;
+	  las.pc = tpmarker->address;
 
-	  sal = find_pc_line (marker->address, 0);
-	  sym = find_pc_sect_function (marker->address, NULL);
+	  las = find_pc_line (tpmarker->address, 0);
+	  sym = find_pc_sect_function (tpmarker->address, NULL);
 	  ui_out_text (uiout, "Now in ");
 	  if (sym)
 	    {
@@ -11667,36 +11667,36 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 				   SYMBOL_PRINT_NAME (sym));
 	      ui_out_text (uiout, " at ");
 	    }
-	  ui_out_field_string (uiout, "file", sal.symtab->filename);
+	  ui_out_field_string (uiout, "file", las.symtab->filename);
 	  ui_out_text (uiout, ":");
 
 	  if (ui_out_is_mi_like_p (uiout))
 	    {
-	      char *fullname = symtab_to_fullname (sal.symtab);
+	      char *fullname = symtab_to_fullname (las.symtab);
 
 	      if (fullname)
 		ui_out_field_string (uiout, "fullname", fullname);
 	    }
 
-	  ui_out_field_int (uiout, "line", sal.line);
+	  ui_out_field_int (uiout, "line", las.line);
 	  ui_out_text (uiout, "\n");
 
-	  b->line_number = sal.line;
+	  b->line_number = las.line;
 
 	  xfree (b->source_file);
 	  if (sym)
-	    b->source_file = xstrdup (sal.symtab->filename);
+	    b->source_file = xstrdup (las.symtab->filename);
 	  else
 	    b->source_file = NULL;
 
 	  xfree (b->addr_string);
 	  b->addr_string = xstrprintf ("%s:%d",
-				       sal.symtab->filename, b->line_number);
+				       las.symtab->filename, b->line_number);
 
 	  /* Might be nice to check if function changed, and warn if
 	     so.  */
 
-	  release_static_tracepoint_marker (marker);
+	  release_static_tracepoint_marker (tpmarker);
 	}
     }
   return sal;
-- 
1.7.5.4

Re: [PATCH 35/358] Fix -Wsahdow warnings

[Tom Tromey]

>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:

Andrey> * breakpoint.c (update_static_tracepoint): Fix -Wshadow
Andrey> warnings.

I think patches 35, 36, 37, and 38 are duplicates of patch 34.

Tom

Re: [PATCH 35/358] Fix -Wsahdow warnings

[Stan Shebs]

On 11/22/11 5:25 AM, Andrey Smirnov wrote:
[...]

-	  struct symtab_and_line sal;
+	  struct symtab_and_line las;


While clever and amusing, this is probably a bad idea for the long run; without any comment explaining it, some future hacker is really really going to want to change it back to "sal", and when enough time has passed, no one will remember why they should object.  Better to use something dull and predictable like "sal2".

Stan
stan@codesourcery.com

Re: [PATCH 35/358] Fix -Wsahdow warnings

[Andrey Smirnov]

On Wed, Nov 23, 2011 at 7:35 AM, Stan Shebs <stanshebs@earthlink.net> wrote:
> On 11/22/11 5:25 AM, Andrey Smirnov wrote:
> [...]
>
> -         struct symtab_and_line sal;
> +         struct symtab_and_line las;
>
>
> While clever and amusing, this is probably a bad idea for the long run;
> without any comment explaining it, some future hacker is really really going
> to want to change it back to "sal", and when enough time has passed, no one
> will remember why they should object.

There are 358 patches and I assure you being clever and amusing did
not come to my mid while I was renaming all conflicting variables. As
to your objection: I thought the goal of the whole patchset is to add
-Wshadow to default compiler flags, is it not? I case it is, than the
future hacker will get a slap from compiler, for shadowing the previous
definition of `sal'. And in case it isn't I have a very loud "Why?!"
to shout at the universe.

>  Better to use something dull and predictable like "sal2".

Given the choice between `las' and `sal2' I have no preference either
way, so I'll rename it.

Andrey Smirnov

[PATCH 034/238] [misc.] Fix -Wshadow warnings.

[Andrey Smirnov]

Cause:
        Local variable shadows function parameter.

To ChangeLog:
	* breakpoint.c (update_static_tracepoint): Rename `sal' to
          `sal2'(-Wshadow).
---
 gdb/ChangeLog    |    5 +++++
 gdb/breakpoint.c |   30 +++++++++++++++---------------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f433ef5..8d1b4dd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2011-11-22  Andrey Smirnov <andrew.smirnov@gmail.com>
 
+	* breakpoint.c (update_static_tracepoint): Fix -Wshadow
+	warnings.
+
+2011-11-22  Andrey Smirnov <andrew.smirnov@gmail.com>
+
 	* breakpoint.c (update_global_location_list): Fix -Wshadow
 	warnings.
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 91a4224..9b359aa 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11640,26 +11640,26 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 
       if (!VEC_empty(static_tracepoint_marker_p, markers))
 	{
-	  struct symtab_and_line sal;
+	  struct symtab_and_line sal2;
 	  struct symbol *sym;
-	  struct static_tracepoint_marker *marker;
+	  struct static_tracepoint_marker *tpmarker;
 	  struct ui_out *uiout = current_uiout;
 
-	  marker = VEC_index (static_tracepoint_marker_p, markers, 0);
+	  tpmarker = VEC_index (static_tracepoint_marker_p, markers, 0);
 
 	  xfree (tp->static_trace_marker_id);
-	  tp->static_trace_marker_id = xstrdup (marker->str_id);
+	  tp->static_trace_marker_id = xstrdup (tpmarker->str_id);
 
 	  warning (_("marker for static tracepoint %d (%s) not "
 		     "found at previous line number"),
 		   b->number, tp->static_trace_marker_id);
 
-	  init_sal (&sal);
+	  init_sal (&sal2);
 
-	  sal.pc = marker->address;
+	  sal2.pc = tpmarker->address;
 
-	  sal = find_pc_line (marker->address, 0);
-	  sym = find_pc_sect_function (marker->address, NULL);
+	  sal2 = find_pc_line (tpmarker->address, 0);
+	  sym = find_pc_sect_function (tpmarker->address, NULL);
 	  ui_out_text (uiout, "Now in ");
 	  if (sym)
 	    {
@@ -11667,36 +11667,36 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 				   SYMBOL_PRINT_NAME (sym));
 	      ui_out_text (uiout, " at ");
 	    }
-	  ui_out_field_string (uiout, "file", sal.symtab->filename);
+	  ui_out_field_string (uiout, "file", sal2.symtab->filename);
 	  ui_out_text (uiout, ":");
 
 	  if (ui_out_is_mi_like_p (uiout))
 	    {
-	      char *fullname = symtab_to_fullname (sal.symtab);
+	      char *fullname = symtab_to_fullname (sal2.symtab);
 
 	      if (fullname)
 		ui_out_field_string (uiout, "fullname", fullname);
 	    }
 
-	  ui_out_field_int (uiout, "line", sal.line);
+	  ui_out_field_int (uiout, "line", sal2.line);
 	  ui_out_text (uiout, "\n");
 
-	  b->line_number = sal.line;
+	  b->line_number = sal2.line;
 
 	  xfree (b->source_file);
 	  if (sym)
-	    b->source_file = xstrdup (sal.symtab->filename);
+	    b->source_file = xstrdup (sal2.symtab->filename);
 	  else
 	    b->source_file = NULL;
 
 	  xfree (b->addr_string);
 	  b->addr_string = xstrprintf ("%s:%d",
-				       sal.symtab->filename, b->line_number);
+				       sal2.symtab->filename, b->line_number);
 
 	  /* Might be nice to check if function changed, and warn if
 	     so.  */
 
-	  release_static_tracepoint_marker (marker);
+	  release_static_tracepoint_marker (tpmarker);
 	}
     }
   return sal;
-- 
1.7.5.4

Re: [PATCH 034/238] [misc.] Fix -Wshadow warnings.

[Joel Brobecker]

> 	* breakpoint.c (update_static_tracepoint): Rename `sal' to
>           `sal2'(-Wshadow).

Looks OK to me. But Pedro might comment on the new variable name
(sal->sal2).

(weird mis-alignment of the ChangeLog, but I assume that'll be fixed
by the time it gets inserted in the ChangeLog file).

> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 91a4224..9b359aa 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -11640,26 +11640,26 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
>  
>        if (!VEC_empty(static_tracepoint_marker_p, markers))
>  	{
> -	  struct symtab_and_line sal;
> +	  struct symtab_and_line sal2;
>  	  struct symbol *sym;
> -	  struct static_tracepoint_marker *marker;
> +	  struct static_tracepoint_marker *tpmarker;
>  	  struct ui_out *uiout = current_uiout;
>  
> -	  marker = VEC_index (static_tracepoint_marker_p, markers, 0);
> +	  tpmarker = VEC_index (static_tracepoint_marker_p, markers, 0);
>  
>  	  xfree (tp->static_trace_marker_id);
> -	  tp->static_trace_marker_id = xstrdup (marker->str_id);
> +	  tp->static_trace_marker_id = xstrdup (tpmarker->str_id);
>  
>  	  warning (_("marker for static tracepoint %d (%s) not "
>  		     "found at previous line number"),
>  		   b->number, tp->static_trace_marker_id);
>  
> -	  init_sal (&sal);
> +	  init_sal (&sal2);
>  
> -	  sal.pc = marker->address;
> +	  sal2.pc = tpmarker->address;
>  
> -	  sal = find_pc_line (marker->address, 0);
> -	  sym = find_pc_sect_function (marker->address, NULL);
> +	  sal2 = find_pc_line (tpmarker->address, 0);
> +	  sym = find_pc_sect_function (tpmarker->address, NULL);
>  	  ui_out_text (uiout, "Now in ");
>  	  if (sym)
>  	    {
> @@ -11667,36 +11667,36 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
>  				   SYMBOL_PRINT_NAME (sym));
>  	      ui_out_text (uiout, " at ");
>  	    }
> -	  ui_out_field_string (uiout, "file", sal.symtab->filename);
> +	  ui_out_field_string (uiout, "file", sal2.symtab->filename);
>  	  ui_out_text (uiout, ":");
>  
>  	  if (ui_out_is_mi_like_p (uiout))
>  	    {
> -	      char *fullname = symtab_to_fullname (sal.symtab);
> +	      char *fullname = symtab_to_fullname (sal2.symtab);
>  
>  	      if (fullname)
>  		ui_out_field_string (uiout, "fullname", fullname);
>  	    }
>  
> -	  ui_out_field_int (uiout, "line", sal.line);
> +	  ui_out_field_int (uiout, "line", sal2.line);
>  	  ui_out_text (uiout, "\n");
>  
> -	  b->line_number = sal.line;
> +	  b->line_number = sal2.line;
>  
>  	  xfree (b->source_file);
>  	  if (sym)
> -	    b->source_file = xstrdup (sal.symtab->filename);
> +	    b->source_file = xstrdup (sal2.symtab->filename);
>  	  else
>  	    b->source_file = NULL;
>  
>  	  xfree (b->addr_string);
>  	  b->addr_string = xstrprintf ("%s:%d",
> -				       sal.symtab->filename, b->line_number);
> +				       sal2.symtab->filename, b->line_number);
>  
>  	  /* Might be nice to check if function changed, and warn if
>  	     so.  */
>  
> -	  release_static_tracepoint_marker (marker);
> +	  release_static_tracepoint_marker (tpmarker);
>  	}
>      }
>    return sal;
> -- 
> 1.7.5.4

-- 
Joel

Re: [PATCH 034/238] [misc.] Fix -Wshadow warnings.

[Andrey Smirnov]

On Mon, Dec 5, 2011 at 6:30 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>>       * breakpoint.c (update_static_tracepoint): Rename `sal' to
>>           `sal2'(-Wshadow).
>
> Looks OK to me. But Pedro might comment on the new variable name
> (sal->sal2).

This name change was suggested by Stan, see
http://sourceware.org/ml/gdb-patches/2011-11/msg00609.html

Initially it was sal->las which probably isn't better, so I'll wait
for some consensus to arise on this patch too.

> (weird mis-alignment of the ChangeLog, but I assume that'll be fixed
> by the time it gets inserted in the ChangeLog file).

I have to edit ChangeLog by hands anyway, so the diffs for it are just
the relics of how I initially made commits. To answer your question:
yes it'll either be fixed or I erroneously weirdly mis-align it even
more :)

Andrey Smirnov

Re: [PATCH 034/238] [misc.] Fix -Wshadow warnings.

[Joel Brobecker]

> > Looks OK to me. But Pedro might comment on the new variable name
> > (sal->sal2).
[...]
> Initially it was sal->las which probably isn't better, so I'll wait
> for some consensus to arise on this patch too.

FWIW, I'm OK with sal2. I also agree with stan that it's best to
avoid `las' (cute, though :-))

-- 
Joel

Re: [PATCH 034/238] [misc.] Fix -Wshadow warnings.

[Pedro Alves]

On Monday 05 December 2011 13:26:46, Joel Brobecker wrote:
> > > Looks OK to me. But Pedro might comment on the new variable name
> > > (sal->sal2).
> [...]
> > Initially it was sal->las which probably isn't better, so I'll wait
> > for some consensus to arise on this patch too.
> 
> FWIW, I'm OK with sal2. I also agree with stan that it's best to
> avoid `las' (cute, though :-))

sal2 is fine with me too.

There might actually be a bug here, that I still haven't managed
to investigate:

 http://sourceware.org/ml/gdb-patches/2011-03/msg00066.html

(but don't let this block your patch.)

Michael wanted sal2 too, so sal2 it is.

Curiously, we have very few instances of struct symtab_and_line
that aren't called "sal" or don't have "sal" in their name:

 $ grep "struct symtab_and_line[ \t]\+[A-Za-z_]\+;" * -rn | grep -v sal
 linespec.c:1910:  struct symtab_and_line val;
 linespec.c:2000:  struct symtab_and_line val;
 mi/mi-cmd-file.c:36:  struct symtab_and_line st;
 symtab.c:1923:  struct symtab_and_line val;

But they're all close enough.  Interestingly, sal2 is the first.  :-)

$ grep sal2 * -rn
*empty*

Sals are funny creatures.

-- 
Pedro Alves