This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] diagnostics: fix end-points of ranges within macros (PR c++/79300)
On Fri, 2017-02-03 at 13:28 -0700, Jeff Law wrote:
> On 02/02/2017 01:53 PM, David Malcolm wrote:
> > PR c++/79300 identifies an issue in which diagnostics_show_locus
> > prints the wrong end-point for a range within a macro:
> >
> > assert ((p + val_size) - buf == encoded_len);
> > ~~~~~~~~~~~~~~~~~~~~~^~~~
> >
> > as opposed to:
> >
> > assert ((p + val_size) - buf == encoded_len);
> > ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> >
> > The caret, start and finish locations of this compound location are
> > all virtual locations.
> >
> > The root cause is that when diagnostic-show-locus.c's layout ctor
> > expands the caret and end-points, it calls
> > linemap_client_expand_location_to_spelling_point
> > which (via expand_location_1) unwinds the macro expansions, and
> > then calls linemap_expand_location. Doing so implicitly picks the
> > *caret* location for any virtual locations, and so in the above
> > case
> > it picks these spelling locations for the three parts of the
> > location:
> >
> > assert ((p + val_size) - buf == encoded_len);
> > ^ ^ ^
> > START | FINISH
> > CARET
> >
> > and so erroneously strips the underlining from the final token,
> > apart
> > from its first character.
> >
> > The fix is for layout's ctor to indicate that it wants the
> > start/finish
> > locations in such a situation, adding a new param to
> > linemap_client_expand_location_to_spelling_point, so that
> > expand_location_1 can handle this case by extracting the relevant
> > part
> > of the unwound compound location, and thus choose:
> >
> > assert ((p + val_size) - buf == encoded_len);
> > ^ ^ ^
> > START | FINISH
> > CARET
> >
> > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
> >
> > OK for stage 4, or should I wait until stage 1?
>
> >
> > gcc/ChangeLog:
> > PR c++/79300
> > * diagnostic-show-locus.c (layout::layout): Use start and
> > finish
> > spelling location for the start and finish of each range.
> > * genmatch.c
> > (linemap_client_expand_location_to_spelling_point):
> > Add unused aspect param.
> > * input.c (expand_location_1): Add "aspect" param, and use it
> > to access the correct part of the location.
> > (expand_location): Pass LOCATION_ASPECT_CARET to new param of
> > expand_location_1.
> > (expand_location_to_spelling_point): Likewise.
> > (linemap_client_expand_location_to_spelling_point): Add
> > "aspect"
> > param, and pass it to expand_location_1.
> >
> > gcc/testsuite/ChangeLog:
> > PR c++/79300
> > * c-c++-common/Wmisleading-indentation-3.c (fn_14): Update
> > expected underlining within macro expansion.
> > * c-c++-common/pr70264.c: Likewise.
> > * g++.dg/plugin/diagnostic-test-expressions-1.C
> > (test_within_macro_1): New test.
> > (test_within_macro_2): Likewise.
> > (test_within_macro_3): Likewise.
> > (test_within_macro_4): Likewise.
> > * gcc.dg/format/diagnostic-ranges.c (test_macro_3): Update
> > expected underlining within macro expansion.
> > (test_macro_4): Likewise.
> > * gcc.dg/plugin/diagnostic-test-expressions-1.c
> > (test_within_macro_1): New test.
> > (test_within_macro_2): Likewise.
> > (test_within_macro_3): Likewise.
> > (test_within_macro_4): Likewise.
> > * gcc.dg/spellcheck-fields-2.c (test_macro): Update expected
> > underlining within macro expansion.
> >
> > libcpp/ChangeLog:
> > PR c++/79300
> > * include/line-map.h (enum location_aspect): New enum.
> > (linemap_client_expand_location_to_spelling_point): Add
> > enum location_aspect param.
> > * line-map.c (source_range::intersects_line_p): Update for new
> > param of linemap_client_expand_location_to_spelling_point.
> > (rich_location::get_expanded_location): Likewise.
> > (fixit_insert::affects_line_p): Likewise.
> I'd suggest waiting.
OK; I plan to self-approve this one in stage 1.
> IIRC you had another issue that was borderline,
> but which we did want to move forward on gcc-7. The reported
> testcase
> was a problem with a new warning, but the underlying issue could (in
> theory) be tripped for existing warnings. RIght?
I think you're talking about PR preprocessor/79210. I found a trivial
way to turn the reproducer for that one into one that ICE-d gcc 7 with
input that gcc 6 could handle, so I marked it as "[7 Regression]",
committed a fix to trunk, and marked it as RESOLVED FIXED.
Dave