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
