On Sat, 2025-02-15 at 16:01 -0500, James K. Lowden wrote:
> From 5d53920602e234e4d99ae2d502e662ee3699978e 4 Oct 2024 12:01:22 -
> 0400
> From: "James K. Lowden" <[email protected]>
> Date: Sat 15 Feb 2025 12:50:53 PM EST
> Subject: [PATCH] 1 new 'cobol' FE file
>
+ if( ($$ & $2) == $2 ) {
+ error_msg(@2, "%s clause repeated", clause);
+ YYERROR;
+ }
Obviously not needed for initial release, but it would be neat to have
a fix-it hint here that deletes the repeated token (fixit hints are
done via class rich_location, FWIW)
+ if( $data_clause == redefines_clause_e ) {
+ error_msg(@2, "REDEFINES must appear "
+ "immediately after LEVEL and NAME");
+ YYERROR;
+ }
A strict reading of our diagnostic guidelines suggests that all of
these keywords in these messages should be in quotes, via %{ and %}, or
via %qs. But given that cobol has UPPERCASE KEYWORDS THAT ALREADY
REALLY STAND OUT, maybe that’s overkill.
+ error_msg(@2, "%s is binary NUMERIC type, "
+ "incompatible with SIGN IS", field->name);
Again, this isn’t needed for the initial release, but GCCs diagnostics
can have “rules” associated with them, which can have URLs (see
diagnostic-metadata.h) Is there a useful public standard for Cobol
with such rules that the output can link to?
+ auto name = nice_name_of($inspected->field);
+ if( !name[0] ) name = "its argument";
+ error_msg(@inspected, "INSPECT cannot write to %s", name);
Building up messages in fragments is a problem for i18n. Better to
have an if/else guarding two separate calls to error_msg. Use %qs in
the one that uses name, so the name appears in quotes.
Not needed for the initial release, but I see a lot of naked “new”,
assigned to $$. Could this be a std::unique_ptr, and use make_unique
rather than new? Similarly for the return type of functions like
new_reference and new_literal. I suspect this would be a lot of work,
though, and may run into snags (does yylval have to a POD?), so no
obligation.
+ if( $a.on_error && $a.not_error ) {
+ error_msg(@b, "too many ON EXCEPTION clauses");
Another thing not needed for the initial release - in general, if we’re
complaining about something in the code being incompatible with other
code we already saw, it’s good to issue a “note” immediately after the
“error”, showing the user the other code so that they can easily see
the two incompatible parts of their code (that’s what class
auto_diagnostic_group is for). But you might not have the source
location available for the other clause; if not, no worries.
Dave
