RFR: 7903779: unify logger messages to show position whenever possible

[Maurizio Cimadamore]

On Thu, 25 Jul 2024 12:28:51 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:

> * logger.warn/.err/.info Position accepting overloads.
> * Error message format is: <position>:"fatal"|"error"|"warning"|"info"|:<message> (consistent with clang).
> * ClangException has position, severity and spelling. ClangExceptions are printed using logger.print.

Looks very good - very nice improvement!

src/main/java/org/openjdk/jextract/impl/Logger.java line 55:

> 53: 
> 54:     public void print(ClangException exp) {
> 55:         errWriter.println(String.format("%1$s: %2$s: %3$s",

In all these cases the "numbered" format argument is probably not necessary, as one can easily see which is which.

src/main/java/org/openjdk/jextract/impl/Logger.java line 59:

> 57:             exp.isFatal()? "fatal" : "error",
> 58:             exp.spelling()));
> 59:         nErrors++;

shouldn't we increase nErrors only based on the severity of the clang diagnostic - e.g. if the diagnostic wrapped by `ClangException` is a warning, then we shouldn't increase `nErrors`. That said, I seem to recollect that the logic in `Parser` only issues a `ClangException` if the severity is > warning. I wonder it might make sense (maybe in a separate PR) to investigate whether adding support for clang warnings might make sense too?

-------------

Marked as reviewed by mcimadamore (Reviewer).

PR Review: https://git.openjdk.org/jextract/pull/254#pullrequestreview-2199256497
PR Review Comment: https://git.openjdk.org/jextract/pull/254#discussion_r1691415442
PR Review Comment: https://git.openjdk.org/jextract/pull/254#discussion_r1691413869