diff mbox

Fix -Wshadow warning in libiberty/cp-demangle.c

Message ID 1478455414.14569.14.camel@klomp.org
State New
Headers show

Commit Message

Mark Wielaard Nov. 6, 2016, 6:03 p.m. UTC 
On Sat, 2016-09-10 at 09:51 -0400, Eric Gallager wrote:
> On 9/10/16, Ian Lance Taylor <iant@google.com> wrote:
> > I'm not sure about the patch to configure.ac/configure.  The last I
> > looked -Wshadow would warn if a local variable shadows a global
> > variable.  That can cause a pointless build break if some system
> > header file defines a global variable that happens to have the same
> > name as a local variable.  It's not a likely scenario but I don't see
> > a need to court a build breakage.
> >
> Maybe if the patch to add -Wshadow-local went in, the configure script
> could use that instead?

We now have -Wshadow=local. So the attached patch uses that for
libiberty. Is the attached patch OK to commit?

Thanks,

Mark

Comments

Mark Wielaard Nov. 13, 2016, 5:47 p.m. UTC | #1 
On Sun, Nov 06, 2016 at 07:03:34PM +0100, Mark Wielaard wrote:
> On Sat, 2016-09-10 at 09:51 -0400, Eric Gallager wrote:
> > On 9/10/16, Ian Lance Taylor <iant@google.com> wrote:
> > > I'm not sure about the patch to configure.ac/configure.  The last I
> > > looked -Wshadow would warn if a local variable shadows a global
> > > variable.  That can cause a pointless build break if some system
> > > header file defines a global variable that happens to have the same
> > > name as a local variable.  It's not a likely scenario but I don't see
> > > a need to court a build breakage.
> > >
> > Maybe if the patch to add -Wshadow-local went in, the configure script
> > could use that instead?
> 
> We now have -Wshadow=local. So the attached patch uses that for
> libiberty. Is the attached patch OK to commit?

Ping?

> From f6f938e8053b6caf393c35c6f96c946073ca9373 Mon Sep 17 00:00:00 2001
> From: Mark Wielaard <mjw@redhat.com>
> Date: Sun, 6 Nov 2016 17:36:06 +0100
> Subject: [PATCH] libiberty: Add -Wshadow=local to warning flags (if
>  supported).
> 
> libiberty/ChangeLog:
> 
>        * configure.ac (ac_libiberty_warn_cflags): Add -Wshadow=local.
>        * configure: Regenerated.
> ---
> diff --git a/libiberty/configure b/libiberty/configure
> index 0f8e9b9..5c4dda5 100755
> --- a/libiberty/configure
> +++ b/libiberty/configure
> @@ -4398,7 +4398,8 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>  ac_libiberty_warn_cflags=
>  save_CFLAGS="$CFLAGS"
>  for real_option in -W -Wall -Wwrite-strings -Wc++-compat \
> -			  -Wstrict-prototypes; do
> +			  -Wstrict-prototypes \
> +			  -Wshadow=local; do
>    # Do the check with the no- prefix removed since gcc silently
>    # accepts any -Wno-* option on purpose
>    case $real_option in
> diff --git a/libiberty/configure.ac b/libiberty/configure.ac
> index 9d3f298..1aa0c7c 100644
> --- a/libiberty/configure.ac
> +++ b/libiberty/configure.ac
> @@ -160,7 +160,8 @@ AC_SYS_LARGEFILE
>  AC_PROG_CPP_WERROR
>  
>  ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wwrite-strings -Wc++-compat \
> -			  -Wstrict-prototypes], [ac_libiberty_warn_cflags])
> +			  -Wstrict-prototypes \
> +			  -Wshadow=local], [ac_libiberty_warn_cflags])
>  ACX_PROG_CC_WARNING_ALMOST_PEDANTIC([], [ac_libiberty_warn_cflags])
>  
>  AC_PROG_CC_C_O
> -- 
> 1.8.3.1
>
Ian Lance Taylor Nov. 14, 2016, 6:27 p.m. UTC | #2 
On Sun, Nov 6, 2016 at 10:03 AM, Mark Wielaard <mark@klomp.org> wrote:
>
> We now have -Wshadow=local. So the attached patch uses that for
> libiberty. Is the attached patch OK to commit?

This is OK.

Thanks.

Ian
diff mbox

Patch

From f6f938e8053b6caf393c35c6f96c946073ca9373 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Sun, 6 Nov 2016 17:36:06 +0100
Subject: [PATCH] libiberty: Add -Wshadow=local to warning flags (if
 supported).

libiberty/ChangeLog:

       * configure.ac (ac_libiberty_warn_cflags): Add -Wshadow=local.
       * configure: Regenerated.
---
diff --git a/libiberty/configure b/libiberty/configure
index 0f8e9b9..5c4dda5 100755
--- a/libiberty/configure
+++ b/libiberty/configure
@@ -4398,7 +4398,8 @@  ac_compiler_gnu=$ac_cv_c_compiler_gnu
 ac_libiberty_warn_cflags=
 save_CFLAGS="$CFLAGS"
 for real_option in -W -Wall -Wwrite-strings -Wc++-compat \
-			  -Wstrict-prototypes; do
+			  -Wstrict-prototypes \
+			  -Wshadow=local; do
   # Do the check with the no- prefix removed since gcc silently
   # accepts any -Wno-* option on purpose
   case $real_option in
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 9d3f298..1aa0c7c 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -160,7 +160,8 @@  AC_SYS_LARGEFILE
 AC_PROG_CPP_WERROR
 
 ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wwrite-strings -Wc++-compat \
-			  -Wstrict-prototypes], [ac_libiberty_warn_cflags])
+			  -Wstrict-prototypes \
+			  -Wshadow=local], [ac_libiberty_warn_cflags])
 ACX_PROG_CC_WARNING_ALMOST_PEDANTIC([], [ac_libiberty_warn_cflags])
 
 AC_PROG_CC_C_O
-- 
1.8.3.1