From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91335 invoked by alias); 20 Sep 2019 19:13:59 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 91326 invoked by uid 89); 20 Sep 2019 19:13:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE autolearn=ham version=3.3.1 spammy=H*c:alternative, quality X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 20 Sep 2019 19:13:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1569006833; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=NgcUOuvZ9JCQ6F47TZ5+4G1UiGXIuGSAru5GYF0JJZk=; b=jUe1f4o9kV7Md6G+PgkVS6W39pjT2gIblU1UQhT/lgpbE1hTFTyjYd3/2HyHUfvPP/Fkhj WGWBd9ySFPTMprtfTZ1uD3V9v7rJmG2ClkFFE7qQKss3fCmBQSDCEYlvAJpigrgSlHhmcg t3clA7UJ9q7YwAnNCfu9xMJoveugTIM= Received: from mail-oi1-f197.google.com (mail-oi1-f197.google.com [209.85.167.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-275-Rks_S470MbiynYTYWRjxeQ-1; Fri, 20 Sep 2019 15:13:51 -0400 Received: by mail-oi1-f197.google.com with SMTP id u69so1754001oia.13 for ; Fri, 20 Sep 2019 12:13:51 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jason Merrill Date: Fri, 20 Sep 2019 19:13:00 -0000 Message-ID: Subject: Re: Adding -Wshadow=local to gcc build rules To: Bernd Edlinger Cc: Richard Biener , gcc Mailing List X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00160.txt.bz2 On Fri, Sep 20, 2019, 2:21 PM Bernd Edlinger wrote: > On 9/19/19 12:19 PM, Richard Biener wrote: > > On Wed, Sep 18, 2019 at 3:09 PM Bernd Edlinger > > wrote: > >> > >> Hi, > >> > >> I'm currently trying to add -Wshadow=3Dlocal to the gcc build rules. > >> I started with -Wshadow, but gave up that idea immediately. > >> > >> As you could expect the current code base has plenty of shadowed > >> local variables. Most are trivial to resolve, some are less trivial. > >> I am not finished yet, but it is clear that it will be a rather big > >> patch. > >> > >> I would like to ask you if you agree that would be a desirable step, > >> in improving code quality in the gcc tree. > > > > I wonder if -Wshadow=3Dcompatible-local is easier to achieve? > > > > I tried that and it does not make a big difference, while > it misses for instance: > > * gcc/c-family/c-ada-spec.c (dump_ada_macros) > unsigned char *s, shadowed by const unsigned char *s. :-/ > > On the other hand, -Wshadow=3Dglobal is a lot more difficult, > because we have lots of globals, for instance: > > context.h: > /* The global singleton context aka "g". > (the name is chosen to be easy to type in a debugger). */ > extern gcc::context *g; > > But unfortunately Wshadow=3Dlocal does not find class members > shadowed by local variable, which happens for instance in > wide-int > > With -Wshadow, I had to change this in wide-int.h: > > Index: gcc/wide-int.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- gcc/wide-int.h (revision 275545) > +++ gcc/wide-int.h (working copy) > @@ -648,9 +648,9 @@ namespace wi > storage_ref () {} > storage_ref (const HOST_WIDE_INT *, unsigned int, unsigned int); > > - const HOST_WIDE_INT *val; > - unsigned int len; > - unsigned int precision; > + const HOST_WIDE_INT *m_val; > + unsigned int m_len; > + unsigned int m_precision; > > > So this change was not necessary for -Wshadow=3Dlocal, although > I would think that, shadowing class members is not much different from > shadowing a local scope. > > Not sure if shadowing class members should be part of -Wshadow=3Dlocal > instead of -Wshadow=3Dglobal. > That would make sense to me. >