From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81213 invoked by alias); 19 Nov 2015 13:58:26 -0000 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 Received: (qmail 81199 invoked by uid 89); 19 Nov 2015 13:58:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 19 Nov 2015 13:58:25 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 5E622C0BFBAA; Thu, 19 Nov 2015 13:58:24 +0000 (UTC) Received: from localhost.localdomain (vpn1-4-82.ams2.redhat.com [10.36.4.82]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAJDwMA5014384; Thu, 19 Nov 2015 08:58:23 -0500 Subject: Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c To: =?UTF-8?Q?Martin_Li=c5=a1ka?= , gcc-patches@gcc.gnu.org, joseph@codesourcery.com References: <564081EF.7030003@suse.cz> <5645DCB1.6070309@suse.cz> <564610B4.9080708@redhat.com> <564616BD.6070206@suse.cz> <564637C9.4000009@redhat.com> <564C89D9.7090303@suse.cz> <564DA193.9030509@suse.cz> From: Bernd Schmidt Message-ID: <564DD57E.1080204@redhat.com> Date: Thu, 19 Nov 2015 13:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <564DA193.9030509@suse.cz> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg02336.txt.bz2 On 11/19/2015 11:16 AM, Martin Liška wrote: > You are right, however as the original coding style was really broken, > it was much easier > to use the tool and clean-up fall-out. > > Waiting for thoughts related to v2. Better, but still some oddities. I hope you won't get mad at me if I suggest doing this in stages? A first patch could just deal with non-reformatting whitespace changes, such as removing trailing whitespace, and converting leading spaces to tabs - that would be mechanical, and reduce the size of the rest of the patch (it seems emacs has an appropriate command, M-x whitespace-cleanup). Such a change is preapproved. A few things I noticed: > - flag_1 = phi <0, 1> // (1) > + flag_1 = phi <0, 1> // (1) Check whether the // (1) is still lined up with the // (2) and // (3) parts. In general I'm not sure we should to what extent we should be reformatting comments in this patch. Breaking long lines and ensuring two spaces after a period seems fine. > + Checking recursively into (1), the compiler can find out that only some_val > + (which is defined) can flow into (3) which is OK. > > */ Could take the opportunity to move the */ onto the end of the line. > + if (is_gimple_call (cond_stmt) && EDGE_COUNT (e->src->succs) >= 2) > + { > + /* Ignore EH edge. Can add assertion > + on the other edge's flag. */ > + continue; > + } Could also take the opportunity to remove excess braces and parentheses. Not a requirement and probably a distraction at this point. > - return (pred.cond_code == NE_EXPR && !pred.invert) > - || (pred.cond_code == EQ_EXPR && pred.invert); > + return (pred.cond_code == NE_EXPR && !pred.invert) > + || (pred.cond_code == EQ_EXPR && pred.invert); This still isn't right. Bernd