Re: [PATCH diagnostics/fortran] Handle two locations for the same diagnostic. Convert all gfc_warning_1 and gfc_notify_std_1 calls

[Manuel López-Ibáñez <lopezibanez at gmail dot com>]

Ping: https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01093.html

On 20 April 2015 at 22:00, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> The Fortran FE allows diagnostics with two different locations.
> Depending on whether these locations are on the same line or not, this
> may produce one or two caret lines. This is the last remaining issue
> left to make Fortran diagnostics use the common code.
>
> In the common diagnostics:
>
> I added support for this in the common diagnostics, although Fortran
> is the only user for now. The new common code should be flexible
> enough to support the Clang style (which I guess is likely to be what
> C/C++ FEs end up supporting sooner or later) while still supporting
> the Fortran style.
>
> Supporting this in the common diagnostics code requires having two
> locations in struct diagnostic_info and two pointers in struct
> text_info. That seems a waste and overtly complex. Thus, I moved the
> new location array directly to struct text_info and added an accessor
> function diagnostic_location().
>
> In addition, this patch factors out the logic to determine whether two
> locations should be shown in the same caret/locus line and the ability
> to just print a caret line in diagnostic_print_caret_line(). These
> functions are used by the common code and the Fortran FE.
>
> In the Fortran FE:
>
> These changes allow me to convert all Fortran FE diagnostic functions
> to use the common code. For simplicity, this patch just converts
> gfc_warning and gfc_notify_std. A follow-up Fortran-only patch will
> convert gfc_error and remove a lot of unused code.
>
> While checking that the new code was working correctly, I noticed that
> the locations (as tracked by line-map) before and after the warning
> given in badline.f were wrong. I fixed this and added two tests to
> check that they are right from now on.
>
> In addition, I added a new function gfc_warning_at to pass an explicit
> location. I think this is better than having another %-code in
> gfc_format_decoder. As you can see in that function, we are now doing
> a lot of work just to print (1) and (2), when the location_t could
> simply be passed explicitly to the diagnostic functions and replace
> all %L and %C with explicit (1) and (2) in the calls. This will remove
> completely gfc_format_decoder. But I'll leave that to Fortran devs if
> they are interested in going that route.
>
> I had to decide what to print for -fno-diagnostics-show-caret and
> multiple locations. It has to be something that can be distinguished
> from a duplicate diagnostic, such that the testsuite can parse it as
> such. I chose to print:
>
> file1:line1:col1: Error: (1)
> file2:line2:col2: Error: message that mentions (1) and (2)
>
> I could print something else if it pleases you, but the above should
> only be seen if someone uses -fno-diagnostics-show-caret explicitly.
>
> Bootstrapped and regression tested on x86_64-linux-gnu.
>
> OK?
>
>
> gcc/fortran/ChangeLog:
>
> 2015-04-19  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     PR fortran/44054
>
>     Replace all calls to gfc_notify_std_1 with gfc_notify_std and
>     gfc_warning_1 with gfc_warning.
>     * decl.c (gfc_verify_c_interop_param): Here.
>     * resolve.c (resolve_branch): Here.
>     (resolve_fl_derived): Here.
>     * dependency.c (gfc_check_argument_var_dependency):
>     * scanner.c (preprocessor_line): Use gfc_warning_now_at. Fix line
>     counter and locations before and after warning.
>     * gfortran.h (gfc_warning_1, gfc_warning_now_1, gfc_notify_std_1):
>     Delete.
>     (gfc_warning_now_at): Declare.
>     * error.c (gfc_warning_1): Delete.
>     (gfc_notify_std_1): Delete.
>     (gfc_warning_now_1): Delete.
>     (gfc_format_decoder): Handle two locations.
>     (gfc_diagnostic_build_prefix): Rename as
>     gfc_diagnostic_build_kind_prefix.
>     (gfc_diagnostic_build_locus_prefix): Take an expanded_location
>     instead of diagnostic_info.
>     (gfc_diagnostic_build_locus_prefix): Add overload that takes two
>     expanded_location.
>     (gfc_diagnostic_starter): Handle two locations.
>     (gfc_warning_now_at): New.
>     (gfc_diagnostics_init): Initialize caret_char array.
>     (gfc_diagnostics_finish): Reset caret_char array to default.
>
>
> gcc/cp/ChangeLog:
>
> 2015-04-19  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     * error.c (cp_diagnostic_starter): Use diagnostic_location
>     function.
>     (cp_print_error_function): Likewise.
>     (cp_printer): Replace locus pointer with location array.
>
> gcc/c/ChangeLog:
>
> 2015-04-19  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     * c-objc-common.c (c_tree_printer): Replace locus pointer with
>     location array.
>
> gcc/ChangeLog:
>
> 2015-04-19  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     * tree-pretty-print.c (percent_K_format): Replace locus pointer
>     with location array.
>     * tree-diagnostic.c (diagnostic_report_current_function): Use
>     diagnostic_location function.
>     (maybe_unwind_expanded_macro_loc): Likewise.
>     (virt_loc_aware_diagnostic_finalizer): Likewise.
>     (default_tree_printer): Replace locus pointer with location array.
>     * diagnostic.c (diagnostic_initialize): Initialize caret_char array.
>     (diagnostic_set_info_translated): Initialize second location.
>     (diagnostic_show_locus): Handle two locations. Call
>     diagnostic_print_caret_line.
>     (diagnostic_print_caret_line): New.
>     (default_diagnostic_starter): Use diagnostic_location function.
>     (diagnostic_report_diagnostic): Use diagnostic_location function.
>     (verbatim): Do not set text.locus.
>     * diagnostic.h (struct diagnostic_info): Remove location field.
>     (struct diagnostic_context): Make caret_char an array of two.
>     (diagnostic_location): New inline.
>     (diagnostic_expand_location): Handle two locations.
>     (diagnostic_same_locus): New inline.
>     (diagnostic_print_caret_line): Declare.
>     * pretty-print.c (pp_printf): Do not set text.locus.
>     (pp_verbatim): Do not set text.locus.
>     * pretty-print.h (struct text_info): Replace locus pointer with
>     location array.
>
> gcc/testsuite/ChangeLog:
>
> 2015-04-19  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     PR fortran/44054
>     * lib/gfortran-dg.exp: Update regex to handle two locations for
>     the same diagnostic without caret.
>     * gfortran.dg/badline.f: Test also that line numbers are correct
>     before and after "left but not entered" warning.