[0/6] v2 of libdiagnostics

  Here's v2 of the "libdiagnostics" shared library idea; see:
  https://gcc.gnu.org/wiki/libdiagnostics

As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public
header file), along with examples of simple self-contained programs that
show various uses of the API.

As in v1, patch 2 (for GCC) is the work-in-progress implementation.

Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing
some syntactic sugar when using the API from C++.  I've been using this
to "eat my own dogfood" and write a simple SARIF-dumping tool:
  https://github.com/davidmalcolm/libdiagnostics-sarif-dump

Patch 4 (for GCC) is an internal change needed by patch 1.

Patch 5 (for GCC) updates GCC's source printing code so that when
there's no column information, we don't print annotation lines.  This
fixes the extra lines seen using it from gas discussed in:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html

Patch 6 (for binutils) is an updated version of the experiment at using
the API from gas.

Thoughts?

David Malcolm (5):
  libdiagnostics v2: header and examples
  libdiagnostics v2: work-in-progress implementation
  libdiagnostics v2: add C++ wrapper API
  diagnostics: add diagnostic_context::get_location_text
  diagnostics: don't print annotation lines when there's no column info

 gcc/Makefile.in                               |  131 +-
 gcc/configure                                 |    2 +-
 gcc/configure.ac                              |    2 +-
 gcc/diagnostic-show-locus.cc                  |   26 +-
 gcc/diagnostic.cc                             |   35 +-
 gcc/diagnostic.h                              |    2 +
 gcc/libdiagnostics++.h                        |  378 +++++
 gcc/libdiagnostics.cc                         | 1306 +++++++++++++++++
 gcc/libdiagnostics.h                          |  602 ++++++++
 gcc/libdiagnostics.map                        |   63 +
 .../libdiagnostics.dg/libdiagnostics.exp      |  544 +++++++
 gcc/testsuite/libdiagnostics.dg/test-dump.c   |   55 +
 .../libdiagnostics.dg/test-error-with-note.c  |   57 +
 .../libdiagnostics.dg/test-error-with-note.cc |   47 +
 gcc/testsuite/libdiagnostics.dg/test-error.c  |   49 +
 gcc/testsuite/libdiagnostics.dg/test-error.cc |   40 +
 .../libdiagnostics.dg/test-fix-it-hint.c      |   49 +
 .../libdiagnostics.dg/test-fix-it-hint.cc     |   44 +
 .../libdiagnostics.dg/test-helpers++.h        |   28 +
 .../libdiagnostics.dg/test-helpers.h          |   29 +
 .../libdiagnostics.dg/test-labelled-ranges.c  |   52 +
 .../libdiagnostics.dg/test-labelled-ranges.cc |   43 +
 .../libdiagnostics.dg/test-logical-location.c |   60 +
 .../libdiagnostics.dg/test-metadata.c         |   54 +
 .../libdiagnostics.dg/test-multiple-lines.c   |   61 +
 .../libdiagnostics.dg/test-no-column.c        |   41 +
 .../test-note-with-fix-it-hint.c              |   52 +
 .../test-text-sink-options.c                  |   46 +
 .../libdiagnostics.dg/test-warning.c          |   52 +
 .../test-write-sarif-to-file.c                |   46 +
 .../test-write-text-to-file.c                 |   47 +
 31 files changed, 4018 insertions(+), 25 deletions(-)
 create mode 100644 gcc/libdiagnostics++.h
 create mode 100644 gcc/libdiagnostics.cc
 create mode 100644 gcc/libdiagnostics.h
 create mode 100644 gcc/libdiagnostics.map
 create mode 100644 gcc/testsuite/libdiagnostics.dg/libdiagnostics.exp
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-dump.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.cc
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.cc
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.cc
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers++.h
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers.h
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.cc
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-logical-location.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-metadata.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-multiple-lines.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-no-column.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-note-with-fix-it-hint.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-text-sink-options.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-warning.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-write-sarif-to-file.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-write-text-to-file.c
  

Thank you for your efforts.
Having the wiki page to track this definitely is useful!

I'll have a look at the "real patch" later, likely next week.

But for patch 4+5 which look quite clean: can we get an early 
improvement and inclusion into GCC for those?
They only adjust internals and should be well covered by the existing 
test suite, so we may be able to inspect the other changes from this 
patchset "alone".

Kind Regards,
Simon

Am 21.11.2023 um 23:20 schrieb David Malcolm:
> Here's v2 of the "libdiagnostics" shared library idea; see:
>    https://gcc.gnu.org/wiki/libdiagnostics
> 
> As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public
> header file), along with examples of simple self-contained programs that
> show various uses of the API.
> 
> As in v1, patch 2 (for GCC) is the work-in-progress implementation.
> 
> Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing
> some syntactic sugar when using the API from C++.  I've been using this
> to "eat my own dogfood" and write a simple SARIF-dumping tool:
>    https://github.com/davidmalcolm/libdiagnostics-sarif-dump
> 
> Patch 4 (for GCC) is an internal change needed by patch 1.
> 
> Patch 5 (for GCC) updates GCC's source printing code so that when
> there's no column information, we don't print annotation lines.  This
> fixes the extra lines seen using it from gas discussed in:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html
> 
> Patch 6 (for binutils) is an updated version of the experiment at using
> the API from gas.
> 
> Thoughts?
> 
> David Malcolm (5):
>    libdiagnostics v2: header and examples
>    libdiagnostics v2: work-in-progress implementation
>    libdiagnostics v2: add C++ wrapper API
>    diagnostics: add diagnostic_context::get_location_text
>    diagnostics: don't print annotation lines when there's no column info
> 
>   gcc/Makefile.in                               |  131 +-
>   gcc/configure                                 |    2 +-
>   gcc/configure.ac                              |    2 +-
>   gcc/diagnostic-show-locus.cc                  |   26 +-
>   gcc/diagnostic.cc                             |   35 +-
>   gcc/diagnostic.h                              |    2 +
>   gcc/libdiagnostics++.h                        |  378 +++++
>   gcc/libdiagnostics.cc                         | 1306 +++++++++++++++++
>   gcc/libdiagnostics.h                          |  602 ++++++++
>   gcc/libdiagnostics.map                        |   63 +
>   .../libdiagnostics.dg/libdiagnostics.exp      |  544 +++++++
>   gcc/testsuite/libdiagnostics.dg/test-dump.c   |   55 +
>   .../libdiagnostics.dg/test-error-with-note.c  |   57 +
>   .../libdiagnostics.dg/test-error-with-note.cc |   47 +
>   gcc/testsuite/libdiagnostics.dg/test-error.c  |   49 +
>   gcc/testsuite/libdiagnostics.dg/test-error.cc |   40 +
>   .../libdiagnostics.dg/test-fix-it-hint.c      |   49 +
>   .../libdiagnostics.dg/test-fix-it-hint.cc     |   44 +
>   .../libdiagnostics.dg/test-helpers++.h        |   28 +
>   .../libdiagnostics.dg/test-helpers.h          |   29 +
>   .../libdiagnostics.dg/test-labelled-ranges.c  |   52 +
>   .../libdiagnostics.dg/test-labelled-ranges.cc |   43 +
>   .../libdiagnostics.dg/test-logical-location.c |   60 +
>   .../libdiagnostics.dg/test-metadata.c         |   54 +
>   .../libdiagnostics.dg/test-multiple-lines.c   |   61 +
>   .../libdiagnostics.dg/test-no-column.c        |   41 +
>   .../test-note-with-fix-it-hint.c              |   52 +
>   .../test-text-sink-options.c                  |   46 +
>   .../libdiagnostics.dg/test-warning.c          |   52 +
>   .../test-write-sarif-to-file.c                |   46 +
>   .../test-write-text-to-file.c                 |   47 +
>   31 files changed, 4018 insertions(+), 25 deletions(-)
>   create mode 100644 gcc/libdiagnostics++.h
>   create mode 100644 gcc/libdiagnostics.cc
>   create mode 100644 gcc/libdiagnostics.h
>   create mode 100644 gcc/libdiagnostics.map
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/libdiagnostics.exp
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-dump.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.cc
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.cc
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.cc
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers++.h
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-helpers.h
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-labelled-ranges.cc
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-logical-location.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-metadata.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-multiple-lines.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-no-column.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-note-with-fix-it-hint.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-text-sink-options.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-warning.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-write-sarif-to-file.c
>   create mode 100644 gcc/testsuite/libdiagnostics.dg/test-write-text-to-file.c
>
  

On 21.11.2023 23:20, David Malcolm wrote:
> @@ -101,6 +109,29 @@ had_warnings (void)
>    return warning_count;
>  }
>  
> +#if USE_LIBDIAGNOSTICS
> +static diagnostic_manager *diag_mgr;
> +#endif
> +
> +void messages_init (void)
> +{
> +#if USE_LIBDIAGNOSTICS
> +  diag_mgr = diagnostic_manager_new ();
> +  diagnostic_manager_add_text_sink (diag_mgr, stderr,
> +				    DIAGNOSTIC_COLORIZE_IF_TTY);

Coloring will want to be possible to turn off (or maybe be off by default,
and be possible to turn on).

As to the #if-s: I think they all would better be #ifdef.

> @@ -182,6 +213,20 @@ as_warn_internal (const char *file, unsigned int line, char *buffer)
>        context = true;
>      }
>  
> +#if USE_LIBDIAGNOSTICS
> +  const diagnostic_file *file_obj
> +    = diagnostic_manager_new_file (diag_mgr, file, NULL);
> +
> +  const diagnostic_physical_location *loc
> +    = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
> +							  file_obj,
> +							  line);
> +
> +  diagnostic *d = diagnostic_begin (diag_mgr,
> +				    DIAGNOSTIC_LEVEL_WARNING);
> +  diagnostic_set_location (d, loc);
> +  diagnostic_finish (d, "%s", buffer);
> +#else

This looks identical to ...

> @@ -256,6 +302,19 @@ as_bad_internal (const char *file, unsigned int line, char *buffer)
>        context = true;
>      }
>  
> +#if USE_LIBDIAGNOSTICS
> +  const diagnostic_file *file_obj
> +    = diagnostic_manager_new_file (diag_mgr, file, NULL);
> +  const diagnostic_physical_location *loc
> +    = diagnostic_manager_new_location_from_file_and_line (diag_mgr,
> +							  file_obj,
> +							  line);
> +
> +  diagnostic *d = diagnostic_begin (diag_mgr,
> +				    DIAGNOSTIC_LEVEL_ERROR);
> +  diagnostic_set_location (d, loc);
> +  diagnostic_finish (d, "%s", buffer);
> +#else

... this, except for the level used. Put into a helper function? Which
would the further want using from as_info_where()?

Jan
  

Hi David,

On 2023-11-21 22:20, David Malcolm wrote:
> Here's v2 of the "libdiagnostics" shared library idea; see:
>   https://gcc.gnu.org/wiki/libdiagnostics
> 
> As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public
> header file), along with examples of simple self-contained programs that
> show various uses of the API.
> 
> As in v1, patch 2 (for GCC) is the work-in-progress implementation.
> 
> Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing
> some syntactic sugar when using the API from C++.  I've been using this
> to "eat my own dogfood" and write a simple SARIF-dumping tool:
>   https://github.com/davidmalcolm/libdiagnostics-sarif-dump
> 
> Patch 4 (for GCC) is an internal change needed by patch 1.
> 
> Patch 5 (for GCC) updates GCC's source printing code so that when
> there's no column information, we don't print annotation lines.  This
> fixes the extra lines seen using it from gas discussed in:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html
> 
> Patch 6 (for binutils) is an updated version of the experiment at using
> the API from gas.
> 
> Thoughts?

Do you have plans on making this a top level library instead?  That would allow easily
making it a non-optional dependency for binutils, as we could have the library in
the binutils-gdb repo as well, for instance.  From the Cauldron discussion I understood that
the diagnostics stuff doesn't depend on much of GCC's data structures, and doesn't rely on
the garbage collector.  Is there something preventing that?  (Other than "it's-a-matter-of-time/effort",
of course.)

Pedro Alves
  

On Tue, 2023-11-21 at 17:20 -0500, David Malcolm wrote:
> gcc/ChangeLog:
>         * diagnostic-show-locus.cc
> (layout::maybe_add_location_range):
>         Don't print annotation lines for ranges when there's no
> column
>         info.
>         (selftest::test_one_liner_no_column): New.
>         (selftest::test_diagnostic_show_locus_one_liner): Call it.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Not a bugfix, but low-risk and useful for libdiagnostics, so I've taken
the liberty of pushing this to gcc trunk as r14-5896-g5099525bff4f7c.

Dave
  

Hi David - and thanks for posting an outline for libdiagnostics at
https://gcc.gnu.org/wiki/libdiagnostics

Currently this shows both libdiagnosts and libdiagnostics-sarif-dump 
integrated into GCC. Is this the plan or would those be available as a 
top-level project (the program as an example for the library), possibly 
with the library sources also pushed to GCC?

Oh, and one question as I stumbled over that today: Would libdiagnostics 
now (or in the future) use libtextstyle for formatting (and another 
possible sink: HTML)?

Simon

Am 23.11.2023 um 18:36 schrieb Pedro Alves:
> Hi David,
> 
> On 2023-11-21 22:20, David Malcolm wrote:
>> Here's v2 of the "libdiagnostics" shared library idea; see:
>>    https://gcc.gnu.org/wiki/libdiagnostics
>>
>> As in v1, patch 1 (for GCC) shows libdiagnostic.h (the public
>> header file), along with examples of simple self-contained programs that
>> show various uses of the API.
>>
>> As in v1, patch 2 (for GCC) is the work-in-progress implementation.
>>
>> Patch 3 (for GCC) adds a new libdiagnostics++.h, a wrapper API providing
>> some syntactic sugar when using the API from C++.  I've been using this
>> to "eat my own dogfood" and write a simple SARIF-dumping tool:
>>    https://github.com/davidmalcolm/libdiagnostics-sarif-dump
>>
>> Patch 4 (for GCC) is an internal change needed by patch 1.
>>
>> Patch 5 (for GCC) updates GCC's source printing code so that when
>> there's no column information, we don't print annotation lines.  This
>> fixes the extra lines seen using it from gas discussed in:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635575.html
>>
>> Patch 6 (for binutils) is an updated version of the experiment at using
>> the API from gas.
>>
>> Thoughts?
> 
> Do you have plans on making this a top level library instead?  That would allow easily
> making it a non-optional dependency for binutils, as we could have the library in
> the binutils-gdb repo as well, for instance.  From the Cauldron discussion I understood that
> the diagnostics stuff doesn't depend on much of GCC's data structures, and doesn't rely on
> the garbage collector.  Is there something preventing that?  (Other than "it's-a-matter-of-time/effort",
> of course.)
> 
> Pedro Alves
> 
>