On 12/14/2016 09:41 AM, Martin Sebor wrote:
- if (i < 0)
+ if (HOST_WIDE_INT_MIN == i)
nit. I think most folks would probably prefer this as
if (i == HOST_WIDE_INT_MIN).
HOST_WIDE_INT_MIN is a constant and when we can write an expression in
either order variable OP const is the preferred order.
You seem to be going back and forth between both styles. Let' try to
stick with variable OP const. I don't think you need to go back and fix
all of the existing const OP variable instances right now, but we may in
the future.
I learned the first style (const OP variable, and also using a less
than in favor of other relational operators) many years ago and I'm
trying to unlearn it for GCC. It's a hard habit to break.
FWIW, the const OP variable style used to be recommended to avoid
subtle bugs due to typos like 'if (var = CST)' But I realize that
with -Wparentheses warning on this there is no need for the style
when using GCC.
Yea. And I can see some argument to use the CST == var style. But
I'm not up to pushing on that in our coding standards. I realize it'll
take time to unlearn :-)
I think the patch for bug 78696 resolves the FIXME (but see below).
If the FIXME was a future thing, then this is OK with the nits fixed. If
the FIXME was a marker for something you intended to address now and
just forgot, then we either need another iteration or a follow-up patch
depending on the severity of the FIXME in your mind.
The patch for bug 78696 resolves the FIXME but there will need to
be another change here to improve the handling of unknown precisions
and widths:
1) Use get_range_info to constrain non-constant width and precision,
and if that fails...
2) ...use some reasonable default (e.g., based on the precision of
the type of the directive).
Without these changes sprintf (d, "%.*f", p, 0.0) causes
warning: writing a terminating nul past the end of the destination
even at -Wformat-length=1 with no good way to suppress it. At
-Wformat-length=2, sprintf(d, "%.*i", 0) also causes a similar:
warning: ‘%.*i’ directive writing 0 or more bytes into a region...
also with no way to suppress it. (The two warnings should also be
worded the same.)
I've started to work on a general fix for both of these in my patch
for bug 78703 - -fprintf-return-value floating point handling
incorrect in locales with a mulltibyte decimal point. These fit
well together because they both separate the heuristic-based warning
byte counters from the actual counters good for optimization (which
are based on what GCC knows for certain).
Understood. Thanks for the update.
jeff
