Bug 56100 - spurious -Wshadow warning with local variable in template class
Summary: spurious -Wshadow warning with local variable in template class
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.2
: P3 normal
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-24 21:11 UTC by Frank Heckenbach
Modified: 2015-04-01 21:37 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-03-31 00:00:00


Attachments
Test case (116 bytes, text/x-c++src)
2013-01-24 21:11 UTC, Frank Heckenbach
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Heckenbach 2013-01-24 21:11:08 UTC
Created attachment 29268 [details]
Test case

Compiling the test case with "-Wshadow" gives:

shadow.cpp: In instantiation of 'void bar<T>::baz() [with T = int]':
shadow.cpp:4:17:   required from 'void bar<T>::qux() [with T = int]'
shadow.cpp:13:21:   required from here
shadow.cpp:6:21: warning: declaration of 'foo' shadows a global declaration [-Wshadow]
shadow.cpp:9:5: warning: shadowed declaration is here [-Wshadow]

Observed with 4.4.6, 4.6.1 and 4.7.2.

Note: "private" is not necessary to trigger the warning, it's just there to show it occurs even with "private".

The warning seems strange because the supposedly shadowed variable is declared after the shadowing one. (I assume that's because of the processing done during instantiation.) It's annoying because it means that an identifier used in a template defined in some header, even in a local scope in a private method is "poisoned" when used anywhere else with -Wshadow.
Comment 1 Andrew Pinski 2013-01-24 21:14:51 UTC
I think this is an artifact of warning during instantiation rather than at definition time.
Comment 2 Frank Heckenbach 2013-01-24 21:25:09 UTC
I guess many warnings can only be given correctly during instantiation because they depend on the actual arguments.

But shadowing is not one of them as the set of identifiers declared doesn't depend on the arguments. (Even in the case of partial specializations, one would expect the warning where the specialization is defined, not when it's instantiated.)

So perhaps a simple fix is to turn off -Wshadow temporarily during instantiation. After all, the instantiated code has no access to currently declared global identifiers. This code fails as it should (AFAIK), unless foo is declared before the template:

template <typename T>
struct bar
{
  int qux () { return foo; }
};

int foo;

int main ()
{
  bar <int> ().qux ();
}
Comment 3 Paolo Carlini 2015-03-31 12:14:50 UTC
I wonder if in such cases would it simply make sense to suppress the warning basing on the locations, eg the below certainly works for the testcase and passes testing:

Index: name-lookup.c
===================================================================
--- name-lookup.c       (revision 221789)
+++ name-lookup.c       (working copy)
@@ -1271,7 +1271,9 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend)
                    }
                }
              else if (oldglobal != NULL_TREE
-                      && (VAR_P (oldglobal)
+                      && ((VAR_P (oldglobal)
+                           && (input_location
+                               > DECL_SOURCE_LOCATION (oldglobal)))
                            /* If the old decl is a type decl, only warn if the
                               old decl is an explicit typedef or if both the
                               old and new decls are type decls.  */
Comment 4 Jason Merrill 2015-03-31 13:15:38 UTC
(In reply to Paolo Carlini from comment #3)
> I wonder if in such cases would it simply make sense to suppress the warning
> basing on the locations

I think we want to suppress the warning on instantiation even if the order is reversed:  given

int foo;

template <typename T>
struct bar
{
  void qux () { baz (); }
private:
  void baz () { int foo; }
};

int main ()
{
  bar <int> ().qux ();
}

We first warn about the declaration in the template, and then again about the instantiation.
Comment 5 Paolo Carlini 2015-03-31 13:55:17 UTC
Doh, thanks Jason I thought I had already checked the behavior in that case.
Comment 6 paolo@gcc.gnu.org 2015-04-01 21:28:27 UTC
Author: paolo
Date: Wed Apr  1 21:27:55 2015
New Revision: 221814

URL: https://gcc.gnu.org/viewcvs?rev=221814&root=gcc&view=rev
Log:
/cp
2015-04-01  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/56100
	* pt.c (instantiating_current_function_p): New.
	* name-lookup.c (pushdecl_maybe_friend_1): Use it.
	* cp-tree.h (instantiating_current_function_p): Declare.

/testsuite
2015-04-01  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/56100
	* g++.dg/warn/Wshadow-8.C: New.
	* g++.dg/warn/Wshadow-9.C: Likewise.
	* g++.dg/warn/Wshadow-10.C: Likewise.
	* g++.dg/warn/Wshadow-11.C: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wshadow-10.C
    trunk/gcc/testsuite/g++.dg/warn/Wshadow-11.C
    trunk/gcc/testsuite/g++.dg/warn/Wshadow-8.C
    trunk/gcc/testsuite/g++.dg/warn/Wshadow-9.C
Modified:
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/name-lookup.c
    trunk/gcc/cp/pt.c
    trunk/gcc/testsuite/ChangeLog
Comment 7 Paolo Carlini 2015-04-01 21:37:58 UTC
Fixed for 5.0.