From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28207 invoked by alias); 24 Jun 2010 19:34:05 -0000 Received: (qmail 28198 invoked by uid 22791); 24 Jun 2010 19:34:04 -0000 X-SWARE-Spam-Status: No, hits=-1.8 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) (74.125.121.35) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 Jun 2010 19:34:00 +0000 Received: from hpaq13.eem.corp.google.com (hpaq13.eem.corp.google.com [172.25.149.13]) by smtp-out.google.com with ESMTP id o5OJXwR3010094 for ; Thu, 24 Jun 2010 12:33:58 -0700 Received: from vws13 (vws13.prod.google.com [10.241.21.141]) by hpaq13.eem.corp.google.com with ESMTP id o5OJXuAQ023088 for ; Thu, 24 Jun 2010 12:33:57 -0700 Received: by vws13 with SMTP id 13so3328691vws.41 for ; Thu, 24 Jun 2010 12:33:56 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.167.133 with SMTP id q5mr37566vcy.254.1277408035640; Thu, 24 Jun 2010 12:33:55 -0700 (PDT) Received: by 10.220.59.77 with HTTP; Thu, 24 Jun 2010 12:33:55 -0700 (PDT) In-Reply-To: References: Date: Thu, 24 Jun 2010 20:22:00 -0000 Message-ID: Subject: Re: [patch] [C/C++] Add a new warning flag -Wself-assign From: Le-Chun Wu To: Paul Koning Cc: GCC Patches Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-06/txt/msg02504.txt.bz2 On Wed, Jun 23, 2010 at 5:54 PM, Paul Koning wrote: >> > Given that self-assign is the documented way to suppress a warning >> (for >> > uninitialized variable) is it appropriate for -Wall to turn this on? >> =A0It >> > would mean that code that was specifically fixed to avoid warnings >> under >> > -Wall will now get warnings again on that exact same line. =A0I don't >> > object to the feature itself, but I do object to that aspect. >> > >> >> This patch actually removes the paragraph in invoke.texi that >> recommends using self-initialization to suppress uninitialized >> variable warnings. Yes, this would be a disruptive change to people >> who use self-initialization, but in my opinion, self-initialization >> doesn't really make sense and is not a good way to fix or work around >> uninitialized variable warnings. (One can initialize a variable with >> other more meaningful value as easily.) > > Yes, and that's usually fine. =A0But there is a performance difference --= a small one admittedly. =A0If the initialization is in fact not necessary,= then the existing coding convention suppresses the warning with no run-tim= e overhead, while the proposed workaround you describe does have a run-time= cost. > I would think most of the uninitialized warnings triggered are genuine bugs and the programmers should just fix them. The self-init workaround was probably used primarily to suppress the false positives that were often caused by the analysis not understanding the predicates that guard the definitions and uses of variables. Now that David Li has checked in the patch that makes the uninitialized variable analysis predicate-aware (r158835), the false positive rate should go down quite a lot. And even for the rare cases where people still have to suppress the warnings, as you said, the performance difference of initializing a variable to a real value should be pretty small (but the code would make more sense). So I still think that we should include -Wself-assign in -Wall, and that people who use self-initialization (with -Wall) should evaluate whether the false positives still exist, and either remove the self-initialization or change the initialization to a more meaningful value. Thanks, Le-chun