Add -fdiagnostics-show-caret to display the context of an error

[lefessan]

When a warning or error is printed, prints the source context with the line of the error, plus 2 lines before and after.
COBC_PREVIEW_ERROR can be used to set the option globally.

[lefessan]

As an example:

prompt> $COMPILE -j -fpreview-error prog.cob
prog.cob:6: error: syntax error, unexpected Identifier

  0004          PROCEDURE        DIVISION.
  0005              DISPLAY "jobby"
  0006 >            EXIT HERE.

[lefessan]

Note that the default is currently to disable this feature, but it could be activated by default (and disabled through COBC_PREVIEW_ERROR=0 for the testsuites)

[GitMensch]

Hm, the idea is good and the general point of reworking the diagnostic format is even on the GSOC idea list for GnuCOBOL (did you get it from there/the note on the dev list about that?). But reading the source file again should be absolute last resort and will be very likely wrong for REPLACE/REPLACING and won't work for complies from stdin. So... Could you please rework that to possibly save the context from the scanner via to a static buffer? Also note that -fno-diagnostics-show-line-numbers should be added along with this change (which would drop the leading line numbers) and possibly -fdiagnostics-show-caret should be used to toggle this instead of -fpreview-error ... and ideally we should also use the caret to show where the error is. As a general rule of thumb we'd like to reach https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html ... someday.

[lefessan]

Hm, the idea is good and the general point of reworking the diagnostic format is even on the GSOC idea list for GnuCOBOL (did you get it from there/the note on the dev list about that?).

No, I didn't know about the GSOC idea. Is it discussed on the forum ?
I did that patch because I needed it myself... which is usually how I work, as I can only devote a small amount of time on the project from time to time.

But reading the source file again should be absolute last resort and will be very likely wrong for REPLACE/REPLACING and won't work for complies from stdin. So... Could you please rework that to possibly save the context from the scanner via to a static buffer?
Also note that -fno-diagnostics-show-line-numbers should be added along with this change (which would drop the leading line numbers) and possibly -fdiagnostics-show-caret should be used to toggle this instead of -fpreview-error ... and ideally we should also use the caret to show where the error is.

I won't be able to spend much more time on this patch. For now, I changed the name of the option to -fdiagnostics-show-caret and added another option to disable it for warnings (typically, we are working on a project where there are currently a lot of warnings, and we want to only focus on errors in a first step).

As a general rule of thumb we'd like to reach https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Message-Formatting-Options.html ... someday.

Would it be ok to merge this work as it is, and wait for the GSOC to get it improved ? I will be working on adding a bit more GCOS support now, so I won't be able to improve this patch more before some time.

[lefessan]

@GitMensch Hi, I say on the forum that you mentioned this patch. Do you think it would be ok to merge it in the current state ?

[GitMensch]

@GitMensch Hi, I've seen on the forum that you mentioned this patch. Do you think it would be ok to merge it in the current state ?

Not in its current state :-/ But after reviewing the initial GCC implementation (also opened the file then read and print the lines), I'm now open to add it without the design change I previously thought of (GCC did that later on).

Things that are likely easy to do:

• drop COBC_DIAGNOSTICS_SHOW_CARET completely; for the testsuite it is enough to add -fno-diagnostics-show-caret in tests/atlocal.in, just as I did with -fno-diagnostics-show-option
• rebase
• add -fno-diagnostics-show-line-numbers (because it is just a single flag that only modifies the output in one place)
• add an entry in NEWS and doc/gnucobol.tex about the two new options

Then, as a follow up, we may use source caches, like the current GCC implementation does.

But still you may want to have a look at the old diagnostic print function linked before your commit and look for good style; and have a look at the current one

[GitMensch]

Nearly ready now - see review details.

[GitMensch]

friendly ping @lefessan

[lefessan]

I think all your requests have been fixed.

[GitMensch]

only minor changes needed (feel free to directly commit)

If you want to finish that completely, then add -fdiagnostics-plain-output to cobc.c (this is a "normal" option, no flag, it has no "no" and would set both new options to ´0`) and use this in tests/atlocal.in for the flags and add it in the NEWS - otherwise I'll do that after your commit.

[GitMensch]

Possibly use in the NEWS something similar to GCCs entry:

the diagnostics now print the source code context with a left margin showing line numbers, configurable with -fno-diagnostics-show-line-numbers, and possible to disable completely with -fno-diagnostics-show-caret;

the option -fdiagnostics-plain-output was added to request that diagnostic output look as plain as possible and stay more stable over time

[GitMensch]

the last commit pushed LGTM

[lefessan]

I think I added all your suggestions, I will commit it to svn as soon as CI is passed. Thanks !

[lefessan]

Merged in SVN

[GitMensch]

Sadly that broke C89 support in cobc/error.c: https://ci.appveyor.com/api/buildjobs/vqa49l488as7peee/log

Please adjust to declare the variables at the beginning of the scope.

Apart from that: what do you think of adding a std=c89 to the CI here? That should have likely called that.

[lefessan]

Sorry, I didn't see your comment, I am going to fix this problem asap

[GitMensch]

Please drop a note if you think you'll have moved around the declarations in the next two hours as I'm doing it otherwise afterwards.

[GitMensch]

Fixing C89 and additional changes to the format and some of the details done "upstream" in svn r5081.

CI PR already referenced.

[GitMensch]

Can you have a look for some late-cleanup on this, moving -fdiagnostics-plain-output from COBC to COBOL_FLAGS, please?
I'd like to keep the command line for NIST, which uses COBC, as small as possible.

--> Note: I'm working on this now.

[GitMensch]

In the meantime I've did that adjustment upstream.