This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] v3 of diagnostic_show_locus and rich_location (was Re: [PATCH 2/5] Reimplement diagnostic_show_locus, introducing rich_location classes (v2))
- From: David Malcolm <dmalcolm at redhat dot com>
- To: Manuel López-Ibáñez <lopezibanez at gmail dot com>
- Cc: Dodji Seketeli <dodji at seketeli dot org>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>, Tobias Burnus <burnus at net-b dot de>, "Joseph S. Myers" <joseph at codesourcery dot com>, Mike Stump <mikestump at comcast dot net>, Rainer Orth <ro at cebitec dot uni-bielefeld dot de>
- Date: Fri, 25 Sep 2015 17:24:47 -0400
- Subject: Re: [PATCH] v3 of diagnostic_show_locus and rich_location (was Re: [PATCH 2/5] Reimplement diagnostic_show_locus, introducing rich_location classes (v2))
- Authentication-results: sourceware.org; auth=none
- References: <1442957171-22904-1-git-send-email-dmalcolm at redhat dot com> <1442957171-22904-3-git-send-email-dmalcolm at redhat dot com> <86h9miswzc dot fsf at seketeli dot org> <1443211881 dot 30732 dot 121 dot camel at surprise> <CAESRpQCFq1rNnJxPkQuZ=Y_vYgqdFXLSDmcz2-gPLt0CX9yxxA at mail dot gmail dot com> <CAESRpQCbZPa8cOYhyJK_u1+ftGuy1kUa2LdtmO7yGfzprSxn0A at mail dot gmail dot com> <CAESRpQArp2fV6miUz8pzMHs6J5mP4X8BWRA3RMFCwrsULMRJYQ at mail dot gmail dot com>
On Fri, 2015-09-25 at 23:13 +0200, Manuel LÃpez-IbÃÃez wrote:
> + If SHOW_CARET_P is true, then the range should be rendered with
> + a caret at its starting location. This
> + is for use by the Fortran frontend, for implementing the
> + "%C" and "%L" format codes. */
> +
> +void
> +rich_location::set_range (unsigned int idx, source_range src_range,
> + bool show_caret_p, bool overwrite_loc_p)
>
> I do not understand when is this show_caret_p used by Fortran given
> the diagnostic_show_locus code mentioned earlier.
The patch is something of a hybrid: on the one hand it's using the new
rich_location class for storing multiple locations for a diagnostic (and
this replaces the existing way we did this in struct text_info), but on
the other hand, for Fortran, it's using the old printing code.
rich_location::set_range exists to ensure that the %C and %L codes used
by Fortran (and "+" in the C family of FEs) can write back into the
rich_location instance, faithfully emulating the old code that wrote
back to
struct text_info's:
location_t locations[MAX_LOCATIONS_PER_MESSAGE];
(that array is replaced in the patch by a rich_location *, pointing back
at the rich_location in the diagnostic_info).
> Related to this:
>
> inline void set_location (unsigned int idx, location_t loc, bool caret_p)
>
> is always called with the last parameter 'true' (boolean parameters
> are always almost bad API). Do you really need this parameter?
Ah, OK. Maybe not there.
> +/* Overwrite the range within this text_info's rich_location.
> + For use e.g. when implementing "+" in client format decoders. */
>
> If we got rid of '+' we would not need this extra work. Also '+'
> breaks #pragma diagnostics. Not the fault of your patch, but it just
> shows that technical debt keeps accumulating.
> https://gcc.gnu.org/wiki/Partial_Transitions
(nods) That "+" thing was one of the surprises I ran into when working
on this, and is the reason that it isn't a :
const rich_location *
but just a:
rich_location *
given that formatting the diagnostic text can lead to the location being
modified. I'm just emulating/supporting the existing behavior.
Dave