On 02/02/2017 09:58 AM, Martin Sebor wrote:
Otherwise all the tests succeeded, though looking e.g. at the diff
between builtin-sprintf-warn-1.c diagnostics with your patch and with
the patch below instead, there are also differences like:
-builtin-sprintf-warn-1.c:1119:3: note: using the range [1, -128] for
directive argument
+builtin-sprintf-warn-1.c:1119:3: note: using the range [-128, 127]
for directive argument
where [1, -128] looks clearly wrong, that isn't a valid range,
The range in the note is the artificial one the pass uses to compute
the range of output. I went back and forth on this as I think it's
debatable if [-128, 127] is clearer than [1, -128] (or [-128, 1]).
The informative notes aren't completely consistent in what ranges
they print. It would be nice to nail down what we think is the most
useful output and make them all consistent.
I don't think there's any argument against moving towards consistency,
but I would well see folks not agreeing on which consistent style is
preferable.
I do think that we should strive to print ranges in the same format that
we use to describe ranges internally. So a range like [1, -128] is not
a valid range. That may argue against using the artificial range
representation in the output.
As for the rest of the patch, while I appreciate your help and
acknowledge it's not my call to make I'm not entirely comfortable
with this much churn at this stage. My preference would to just
fix the bugs and do the restructuring in stage 1.
I'm really torn here. I'm keen to raise the bar in terms of what we're
willing to do for gcc-7. But I'm also keen to have the codebase in a
reasonable state that will allow for the ongoing maintenance of the
gcc-7 branch.
The sprintf stuff is fairly dense and there's almost certainly more
dusty corner case issues we're going to have to work through. Thus we
stand to be in a better state to maintain the code if we can do some
refactoring.
The fact that Jakub, one of the release managers, is proposing the
change carries a lot of weight in terms of trying to make a decision
here. Similarly your weight as the author of this code carries a lot of
weight. Leaving us without a clearly preferable path.