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.


Jeff

Reply via email to