On Sun, 2025-11-30 at 02:06 +0100, Jose E. Marchesi wrote: > This commit adds the diagnostics infrastructure for the Algol 68 > front-end.
Hi Jose, congrats on getting all this merged. A few comments inline below: > > Signed-off-by: Jose E. Marchesi <[email protected]> > Co-authored-by: Marcel van der Veer <[email protected]> > > gcc/ChangeLog > > * algol68/a68-diagnostics.cc: New file. > --- > gcc/algol68/a68-diagnostics.cc | 381 > +++++++++++++++++++++++++++++++++ > 1 file changed, 381 insertions(+) > create mode 100644 gcc/algol68/a68-diagnostics.cc > > diff --git a/gcc/algol68/a68-diagnostics.cc b/gcc/algol68/a68- > diagnostics.cc > new file mode 100644 > index 00000000000..0c25da2a21f > --- /dev/null > +++ b/gcc/algol68/a68-diagnostics.cc > @@ -0,0 +1,381 @@ > +/* Error and warning routines. > + Copyright (C) 2001-2023 J. Marcel van der Veer. > + Copyright (C) 2025 Jose E. Marchesi. > + > + Original implementation by J. Marcel van der Veer. > + Adapted and expanded for GCC by Jose E. Marchesi. > + > + 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 > + <http://www.gnu.org/licenses/>. */ > + > +#define INCLUDE_MEMORY > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "diagnostic.h" > + > +#include "a68.h" > + > +/* > + * Error handling routines. > + */ > + > +#define TABULATE(n) (8 * (n / 8 + 1) - n) > + > +/* Severities handled by the DIAGNOSTIC function defined below. */ > + > +#define A68_ERROR 0 > +#define A68_WARNING 1 > +#define A68_FATAL 2 > +#define A68_SCAN_ERROR 3 > +#define A68_INFORM 4 > + > +/* Give a diagnostic message. */ > + > +#if __GNUC__ >= 10 > +#pragma GCC diagnostic ignored "-Wsuggest-attribute=format" > +#endif > + > +static bool > +diagnostic (int sev, int opt, > + NODE_T *p, > + LINE_T *line, > + char *pos, > + const char *loc_str, va_list args) > +{ > + int res = 0; > + MOID_T *moid = NO_MOID; > + const char *t = loc_str; > + obstack b; > + > + /* > + * Synthesize diagnostic message. > + * > + * Legend for special symbols: > + * * as first character, copy rest of string literally > + * @ AST node > + * A AST node attribute > + * B keyword > + * C context > + * L line number > + * M moid - if error mode return without giving a message > + * O moid - operand > + * S quoted symbol, when possible with typographical display > features > + * X expected attribute > + * Y string literal. > + * Z quoted string. */ Out of curiosity, is it possible to implement all of this via a pp_format_decoder callback, rather than generating a format string for use by pp_format? [...snip...] > + > + switch (att) > + { > + case NO_SORT: sort = "this"; break; > + case SOFT: sort = "a soft"; break; > + case WEAK: sort = "a weak"; break; > + case MEEK: sort = "a meek"; break; > + case FIRM: sort = "a meek"; break; Should "sort" be "a firm" in "case FIRM:" above? (copy-and-paste error?) > + case STRONG: sort = "a strong"; break; > + default: > + gcc_unreachable (); > + } > + > + obstack_grow (&b, sort, strlen (sort)); > + } > + else if (t[0] == 'L') > + { > + LINE_T *a = va_arg (args, LINE_T *); > + gcc_assert (a != NO_LINE); > + if (NUMBER (a) == 0) > + obstack_grow (&b, "in standard environment", > + strlen ("in standard environment")); > + else if (p != NO_NODE && NUMBER (a) == LINE_NUMBER (p)) > + obstack_grow (&b, "in this line", strlen ("in this > line")); > + else > + { > + char d[10]; > + if (snprintf (d, 10, "in line %d", NUMBER (a)) < 0) > + gcc_unreachable (); If I'm counting correctly, "in line " is 8 chars, so leaving only one char for the number, and one for the nul-terminator, which suggests NUMBER (a) can only be a single digit. Is that correct, or should the size be increased from 10? (used in both d and the snprintf call). > + obstack_grow (&b, d, strlen (d)); > + } [...snip...] > + obstack_grow (&b, "UNION (VOID, ..)", strlen ("UNION > (VOID, ..)")); There's a lot of obstack_grow (&b, str, strlen (str); in this code where we hope that "UNION (VOID, ..)" is the same string as "UNION (VOID, ..)". How about a helper function, something like: obstack_append_str (&b, str); (or a macro, if you're trying to avoid the runtime-computation of strlen, but this is diagnostic code and so almost certainly not a hot- spot in the profile). [...snip...] Hope this is constructive Dave
