Re: [PATCH 1/2] libdiagnostics: header and examples

[David Malcolm]

On Tue, 2023-11-07 at 19:02 -0500, Lewis Hyatt wrote:
> On Mon, Nov 6, 2023 at 8:29 PM David Malcolm 
> wrote:
> > 
> > Here's a work-in-progress patch for GCC that adds a
> > libdiagnostics.h
> > header describing the public interface, along with various
> > testcases
> > that show usage examples for the API.  Various aspects of this need
> > work; posting now for early feedback on overall direction.
> > 
> > How does the interface look?
> > 
> ...
> > +typedef unsigned int diagnostic_location_t;
> 
> One comment that occurred to me... for GCC we have a lot of PRs that
> are unhappy about the 32-bit location_t and the consequent issues
> that
> arise with very large source files, or with very long lines that lose
> column information.
> So far GCC has been able to get by with "don't do that" advice, but a
> more general libdiagnostics may need to avoid that arbitrary
> limitation? I feel like it may not be that long before GCC needs to
> deal with it as well, perhaps with a configure option, but even now,
> it could make sense for libdiagnostic to use a 64-bit location_t
> itself from the outset, so it won't need to change later, even if
> it's
> practically restricted to 32 bits for now.

That's a good point.

Perhaps the interface should give back a pointer to an opaque type, so
it would be:

  typedef struct diagnostic_location diagnostic_location;

and e.g.

extern const diagnostic_location *
diagnostic_manager_new_location_from_file_and_line (diagnostic_manager 
*diag_mgr,
const diagnostic_file *file,
diagnostic_line_num_t 
line_num);


where the diagnostic_manager owns the underlying memory.

Dave

Re: [PATCH 1/2] libdiagnostics: header and examples

[Lewis Hyatt]

On Mon, Nov 6, 2023 at 8:29 PM David Malcolm  wrote:
>
> Here's a work-in-progress patch for GCC that adds a libdiagnostics.h
> header describing the public interface, along with various testcases
> that show usage examples for the API.  Various aspects of this need
> work; posting now for early feedback on overall direction.
>
> How does the interface look?
>
...
> +typedef unsigned int diagnostic_location_t;

One comment that occurred to me... for GCC we have a lot of PRs that
are unhappy about the 32-bit location_t and the consequent issues that
arise with very large source files, or with very long lines that lose
column information.
So far GCC has been able to get by with "don't do that" advice, but a
more general libdiagnostics may need to avoid that arbitrary
limitation? I feel like it may not be that long before GCC needs to
deal with it as well, perhaps with a configure option, but even now,
it could make sense for libdiagnostic to use a 64-bit location_t
itself from the outset, so it won't need to change later, even if it's
practically restricted to 32 bits for now.

-Lewis

[PATCH 1/2] libdiagnostics: header and examples

[David Malcolm]

Here's a work-in-progress patch for GCC that adds a libdiagnostics.h
header describing the public interface, along with various testcases
that show usage examples for the API.  Various aspects of this need
work; posting now for early feedback on overall direction.

How does the interface look?

gcc/ChangeLog:
* libdiagnostics.h: New file.

gcc/testsuite/ChangeLog:
* libdiagnostics.dg/test-error-with-note.c: New test.
* libdiagnostics.dg/test-error.c: New test.
* libdiagnostics.dg/test-fix-it-hint.c: New test.
* libdiagnostics.dg/test-helpers.h: New.
* libdiagnostics.dg/test-labelled-ranges.c: New test.
* libdiagnostics.dg/test-logical-location.c: New test.
* libdiagnostics.dg/test-metadata.c: New test.
* libdiagnostics.dg/test-multiple-lines.c: New test.
* libdiagnostics.dg/test-note-with-fix-it-hint.c: New test.
* libdiagnostics.dg/test-warning.c: New test.
* libdiagnostics.dg/test-write-sarif-to-file.c: New test.
* libdiagnostics.dg/test-write-text-to-file.c: New test.
---
 gcc/libdiagnostics.h  | 544 ++
 .../libdiagnostics.dg/test-error-with-note.c  |  57 ++
 gcc/testsuite/libdiagnostics.dg/test-error.c  |  49 ++
 .../libdiagnostics.dg/test-fix-it-hint.c  |  48 ++
 .../libdiagnostics.dg/test-helpers.h  |  29 +
 .../libdiagnostics.dg/test-labelled-ranges.c  |  52 ++
 .../libdiagnostics.dg/test-logical-location.c |  62 ++
 .../libdiagnostics.dg/test-metadata.c |  53 ++
 .../libdiagnostics.dg/test-multiple-lines.c   |  58 ++
 .../test-note-with-fix-it-hint.c  |  51 ++
 .../libdiagnostics.dg/test-warning.c  |  52 ++
 .../test-write-sarif-to-file.c|  46 ++
 .../test-write-text-to-file.c |  47 ++
 13 files changed, 1148 insertions(+)
 create mode 100644 gcc/libdiagnostics.h
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error-with-note.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-error.c
 create mode 100644 gcc/testsuite/libdiagnostics.dg/test-fix-it-hint.c
 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-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-note-with-fix-it-hint.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

diff --git a/gcc/libdiagnostics.h b/gcc/libdiagnostics.h
new file mode 100644
index 000..672594598fa
--- /dev/null
+++ b/gcc/libdiagnostics.h
@@ -0,0 +1,544 @@
+/* A pure C API for emitting diagnostics.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#ifndef LIBDIAGNOSTICS_H
+#define LIBDIAGNOSTICS_H
+
+/* We use FILE * for streams */
+#include 
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+/**
+ Macros for attributes.
+ These are all currently empty, and thus for the human reader rather than
+ the compiler.
+ **/
+
+#define LIBDIAGNOSTICS_PARAM_MUST_BE_NON_NULL(ARG_NUM)
+
+#define LIBDIAGNOSTICS_PARAM_CAN_BE_NULL(ARG_NUM)
+
+#define LIBDIAGNOSTICS_PARAM_GCC_FORMAT_STRING(FMT_ARG_NUM, ARGS_ARG_NUM)
+
+
+/**
+ Data structures and types.
+ All structs within the API are opaque.
+ **/
+
+/* An opaque bundle of state for a client of the library.
+   Has zero of more "sinks" to which diagnostics are emitted.
+   Responsibilities:
+   - location-management
+   - caching of source file content
+   - patch generation.  */
+typedef struct diagnostic_manager diagnostic_manager;
+
+/* Types relating to diagnostic output sinks.  */
+
+/* An enum for determining if we should colorize a text output sink.  */
+enum diagnostic_colorize
+{
+