From patchwork Thu Jun 10 22:37:33 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Le-Chun Wu X-Patchwork-Id: 55275 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2AB5BB7D6D for ; Fri, 11 Jun 2010 08:37:59 +1000 (EST) Received: (qmail 16178 invoked by alias); 10 Jun 2010 22:37:56 -0000 Received: (qmail 15723 invoked by uid 22791); 10 Jun 2010 22:37:52 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Jun 2010 22:37:38 +0000 Received: from wpaz24.hot.corp.google.com (wpaz24.hot.corp.google.com [172.24.198.88]) by smtp-out.google.com with ESMTP id o5AMbZN0010557 for ; Thu, 10 Jun 2010 15:37:35 -0700 Received: from pxi18 (pxi18.prod.google.com [10.243.27.18]) by wpaz24.hot.corp.google.com with ESMTP id o5AMbYoS002139 for ; Thu, 10 Jun 2010 15:37:34 -0700 Received: by pxi18 with SMTP id 18so177094pxi.22 for ; Thu, 10 Jun 2010 15:37:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.141.106.8 with SMTP id i8mr604882rvm.275.1276209454005; Thu, 10 Jun 2010 15:37:34 -0700 (PDT) Received: by 10.141.124.12 with HTTP; Thu, 10 Jun 2010 15:37:33 -0700 (PDT) In-Reply-To: <4C09C96A.3020506@redhat.com> References: <4BFEC1B9.2050702@redhat.com> <4C09C96A.3020506@redhat.com> Date: Thu, 10 Jun 2010 15:37:33 -0700 Message-ID: Subject: Re: [patch] fix PR/44128 (C++ not warn on type decl shadowing with -Wshadow) From: Le-Chun Wu To: Jason Merrill Cc: GCC Patches , Mark Mitchell , neil@daikokuya.co.uk X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Fri, Jun 4, 2010 at 8:50 PM, Jason Merrill wrote: > On 06/04/2010 09:30 PM, Le-Chun Wu wrote: >> >> On Thu, May 27, 2010 at 12:02 PM, Jason Merrill  wrote: > >>> Why only warn if the TREE_CODE happens to match?  I think we might as >>> well >>> also warn if, for instance, a typedef shadows a variable. > >> The current implementation (before my change) already warns if a >> typedef shadows a variable. What it doesn't warn is if a local >> variable shadows a typedef. And this behavior appears to be by design >> (based on PR/16). The rationale provided in PR/16 is that when a local >> variable shadows a previously-declared struct/class/enum name, it's >> probably not accidental but intentional, and therefore the compiler >> should not warn about it. > > Yes, but that's because struct/class/enum names can be hidden by other > declarations in the same scope, for C compatibility.  We should still warn > about shadowing an explicit typedef. > Sure.. I have revised the patch so that the compiler warns if 1. a local variable shadows another variable, parameter, class member, or explicit typedef (but not warn if shadows a struct/class/enum name) 2. a type declaration (explicit typedef, struct, class, enum) shadows another variable, parameter, class member, or type declaration. This way, we expand the warning to cover type declarations but still preserve the fix for PR/16. What do you think? Thanks, Le-chun 2010-06-10 Le-Chun Wu PR/44128 * gcc/doc/invoke.texi: Update documentation of -Wshadow. * gcc/cp/name-lookup.c (pushdecl_maybe_friend): Warn when a local decl shadows another local or global decl if both decls have the same TREE_CODE. * gcc/testsuite/g++.dg/warn/Wshadow-7.C: New test. Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 160582) +++ gcc/doc/invoke.texi (working copy) @@ -3842,8 +3842,10 @@ Do not warn whenever an @samp{#else} or @item -Wshadow @opindex Wshadow @opindex Wno-shadow -Warn whenever a local variable shadows another local variable, parameter or -global variable or whenever a built-in function is shadowed. +Warn whenever a local variable or type declaration shadows another variable, +parameter, type, or class member (in C++), or whenever a built-in function +is shadowed. Note that in C++, the compiler will not warn if a local variable +shadows a struct/class/enum, but will warn if shadows an explicit typedef. @item -Wlarger-than=@var{len} @opindex Wlarger-than=@var{len} Index: gcc/testsuite/g++.dg/warn/Wshadow-7.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wshadow-7.C (revision 0) +++ gcc/testsuite/g++.dg/warn/Wshadow-7.C (revision 0) @@ -0,0 +1,37 @@ +// PR c++/44128 +// { dg-options "-Wshadow" } + +typedef long My_ssize_t; // { dg-warning "shadowed declaration" } +typedef int Foo; // { dg-warning "shadowed declaration" } +struct Bar1 { // { dg-bogus "shadowed declaration" } + int a; +}; +struct Bar2 { // { dg-warning "shadowed declaration" } + int a; +}; + +void func() { + typedef int My_ssize_t; // { dg-warning "shadows a global" } + typedef char My_Num; // { dg-warning "shadowed declaration" } + { + typedef short My_Num; // { dg-warning "shadows a previous local" } + } + int Foo; // { dg-warning "shadows a global" } + float Bar1; // { dg-bogus "shadows a global" } + struct Bar2 { // { dg-warning "shadows a global" } + int a; + }; + struct Bar3 { // { dg-warning "shadowed declaration" } + int a; + }; + struct Bar4 { // { dg-bogus "shadowed declaration" } + int a; + }; + { + struct Bar3 { // { dg-warning "shadows a previous local" } + int a; + }; + char Bar4; // { dg-bogus "shadows a previous local" } + int My_Num; // { dg-warning "shadows a previous local" } + } +} Index: gcc/cp/name-lookup.c =================================================================== --- gcc/cp/name-lookup.c (revision 160582) +++ gcc/cp/name-lookup.c (working copy) @@ -1017,10 +1017,22 @@ pushdecl_maybe_friend (tree x, bool is_f /* Inline decls shadow nothing. */ && !DECL_FROM_INLINE (x) && (TREE_CODE (oldlocal) == PARM_DECL - || TREE_CODE (oldlocal) == VAR_DECL) - /* Don't check the `this' parameter. */ - && !DECL_ARTIFICIAL (oldlocal) - && !DECL_ARTIFICIAL (x)) + || TREE_CODE (oldlocal) == VAR_DECL + /* 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. */ + || (TREE_CODE (oldlocal) == TYPE_DECL + && (!DECL_ARTIFICIAL (oldlocal) + || TREE_CODE (x) == TYPE_DECL))) + /* Don't check the `this' parameter or internally generated + vars unless it's an implicit typedef (see + create_implicit_typedef in decl.c). */ + && (!DECL_ARTIFICIAL (oldlocal) + || DECL_IMPLICIT_TYPEDEF_P (oldlocal)) + /* Don't check for internally generated vars unless + it's an implicit typedef (see create_implicit_typedef + in decl.c). */ + && (!DECL_ARTIFICIAL (x) || DECL_IMPLICIT_TYPEDEF_P (x))) { bool nowarn = false; @@ -1081,10 +1093,12 @@ pushdecl_maybe_friend (tree x, bool is_f /* Maybe warn if shadowing something else. */ else if (warn_shadow && !DECL_EXTERNAL (x) - /* No shadow warnings for internally generated vars. */ - && ! DECL_ARTIFICIAL (x) - /* No shadow warnings for vars made for inlining. */ - && ! DECL_FROM_INLINE (x)) + /* No shadow warnings for internally generated vars unless + it's an implicit typedef (see create_implicit_typedef + in decl.c). */ + && (! DECL_ARTIFICIAL (x) || DECL_IMPLICIT_TYPEDEF_P (x)) + /* No shadow warnings for vars made for inlining. */ + && ! DECL_FROM_INLINE (x)) { tree member; @@ -1103,7 +1117,13 @@ pushdecl_maybe_friend (tree x, bool is_f x); } else if (oldglobal != NULL_TREE - && TREE_CODE (oldglobal) == VAR_DECL) + && (TREE_CODE (oldglobal) == VAR_DECL + /* 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. */ + || (TREE_CODE (oldglobal) == TYPE_DECL + && (!DECL_ARTIFICIAL (oldglobal) + || TREE_CODE (x) == TYPE_DECL)))) /* XXX shadow warnings in outer-more namespaces */ { warning_at (input_location, OPT_Wshadow,