Re: [PATCH] Fix various minor gimple-ssa-sprintf.c issues

[Marek Polacek <polacek@redhat.com>]

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.

	Marek