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
