From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116546 invoked by alias); 23 Sep 2016 17:42:38 -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 116527 invoked by uid 89); 23 Sep 2016 17:42:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=trained, hundred, eye X-HELO: mail-yw0-f194.google.com Received: from mail-yw0-f194.google.com (HELO mail-yw0-f194.google.com) (209.85.161.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Sep 2016 17:42:36 +0000 Received: by mail-yw0-f194.google.com with SMTP id t67so6211652ywg.1 for ; Fri, 23 Sep 2016 10:42:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:cc:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Wd6rv/8VoMfEdCKTcxMPvUpUcTI63enX3NOmhuDDZ4M=; b=SmSShQat+ZaZUkJah6byYjOaqzPp9VNAZBLKmKhalXVMwtDpRzv3LqgOhHQoElXJww eXRepZFPB87CVEVucU3Vn9l1Tghu/jJh466DsBerEL/3yUC3fhPGEjT2ekYINiueqMSC JnAiQc8Ru3mmSzykhF4lS+qUTobhY3itfNaKmwWn988PFN5lnvLGjgptwI/i8JmJbJjn R+wbXHdrokLsk1Pz02IE6lN6uzCG0AXkSV79cTTAMwhn7X5q5P/nkea+s3IGcafqsf3t QYYBgdPkuyHydvpnUpyGhnrTylv4cGl05Hkx3DByra13PSnjLCUMkT35H1i66PZiM3T0 IspQ== X-Gm-Message-State: AE9vXwPJQBpfmhqnYUy1mcc23b8TNqUP1ZoTM0qbgmDBRtyxQDjytEAROZ5TsIHjxsNXMQ== X-Received: by 10.13.233.194 with SMTP id s185mr6665657ywe.191.1474652554337; Fri, 23 Sep 2016 10:42:34 -0700 (PDT) Received: from [192.168.0.26] (75-166-199-51.hlrn.qwest.net. [75.166.199.51]) by smtp.gmail.com with ESMTPSA id p203sm3122004ywb.14.2016.09.23.10.42.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Sep 2016 10:42:33 -0700 (PDT) From: Martin Sebor Subject: Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues To: Marek Polacek , Jakub Jelinek References: <20160921150954.GC7282@tucnak.redhat.com> <7f35dddc-af39-1329-cca6-33a88955a469@gmail.com> <20160922072411.GF7282@tucnak.redhat.com> <20160922074619.GS19950@redhat.com> Cc: Jeff Law , gcc-patches@gcc.gnu.org Message-ID: <7f46c146-4527-0bb7-ec94-c8f2c83b0c68@gmail.com> Date: Fri, 23 Sep 2016 17:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20160922074619.GS19950@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg01728.txt.bz2 On 09/22/2016 01:46 AM, Marek Polacek wrote: > On Thu, Sep 22, 2016 at 09:24:11AM +0200, Jakub Jelinek wrote: >> On Wed, Sep 21, 2016 at 06:38:54PM -0600, Martin Sebor wrote: >>> On 09/21/2016 09:09 AM, Jakub Jelinek wrote: >>>> When looking at PR77676, I've noticed various small formatting etc. >>>> issues, like not using is_gimple_* APIs where we have them, not using >>>> gimple_call_builtin_p/gimple_call_fndecl (this one actually can show up, >>>> if e.g. uses the builtin with incorrect arguments (fewer, different >>>> types etc.)), one pasto, 2 spaces in comments instead of 1 in the middle >>>> of sentences. And, lastly 0 < var is very unusual ordering of the >>>> comparison operands, while we have a couple of such cases in the sources, >>>> usually it is when using 0 < var && var <= someotherconst, while >>>> var > 0 is used hundred times more often. >>> >>> Thanks for correcting the uses of the gimple APIs! I appreciate >>> your fixing the various typos as well, but I see no value in >>> changing the order of operands in inequality expressions or in >>> moving code around for no apparent reason. However, I won't >> >> The moving of code around is in just one spot, and it has a reason - >> consistency. After the move, each non-_chk builtin is followed by its _chk >> counterpart, before that the order has been random. >> Another possible ordering that makes sense is putting all the non-_chk >> builtins first and then in the same order all their _chk counterparts. >> >> The reason why I wrote the patch has been that when skimming the code I've >> noticed the missing is_* calls, then when looking for that issue discovered >> something different etc. The var > 0 vs. 0 < var is just something that >> caught my eye when looking around, I don't feel too strongly about it, it >> just looked weird and unexpected. There are > 50 optimize > 0 preexisting >> checks elsewhere, and even far more just optimize, but none 0 < optimize. > > I find those 0 < var confusing and hard to read. While I know that some > people prefer 0 == var (0 is not an lvalue so it catches mistakes like > var = 0 instead of var == 0), I don't see why 0 < optimized would ever be > preferred. I don't have a preference for one or the other. It's just how I wrote the code. Over the years of working on the C++ standard library I trained myself to use the less than expression in preference to any of the others because C++ algorithms had to (or we wanted them to, I don't remember which anymore) work with user-defined types that only defined that one relational operator and not any of the other forms. Martin