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
