On 11/18/2016 03:57 PM, David Malcolm wrote:
On Fri, 2016-11-18 at 09:51 -0700, Martin Sebor wrote:
Martin: are the changes to your test cases OK by you, or is there
a better way to rewrite them?
Thanks for looking into it!
Since the purpose of the test_sprintf_note function in the test is
to verify the location of the caret within the warnings I think we
should keep it if it's possible. Would either removing the P macro
or moving the function to a different file that doesn't use the
-ftrack-macro-expansion=0 option work?
To get substring locations with the proposed patch, that code will need to
be in a file without -ftrack-macro-expansion=0.
builtin-sprintf-warn-4.c seems to fit the bill, and has caret-printing
enabled too, so here's another attempt at the patch, just covering the
affected test cases, which moves test_sprintf_note to that file, and
drops "P".
The carets/underlines from the warnings look sane, and the test case
verifies that via dg-begin/end-multiline-output directives. The test
case also verifies the carets/underlins from the *notes*.
[FWIW, I'm less convinced by the carets/underlines from the notes:
they all underline the whole of the __builtin_sprintf expression,
though looking at gimple-ssa-sprintf.c, I see:
location_t callloc = gimple_location (info.callstmt);
which is used for the "inform" calls in question. Hence I think
it's faithfully printing what that code is asking it to. I'd prefer
to not touch that location in this patch, since it seems
orthogonal to fixing the PR preprocessor/78324; perhaps something
to address as part of PR middle-end/77696 ?].
Martin: how does this look to you? Any objections to this change
as part of my fix for PR preprocessor/78324?
Not at all. It looks great -- using the multiline output is even
better than the original. You also noticed the comment about the
caret limitation being out of data and removed it. Thanks for
that too!
I agree that the underlining in the notes could stand to be
improved at some point. I'll see if I can get to it sometime
after I'm done with all my pending patches.
Thanks again!
Martin
PS If there's something I can help with while you're working on
the rest of the bug let me know.
