[foreign-jextract] [Rev 01] RFR: Improve jextract error reporting

[Athijegannathan Sundararajan]

On Fri, 6 Mar 2020 11:00:34 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Hi,
>> 
>> I spent the last couple of days tracking down a crash I was seeing sometimes when running jextract. In the process I ended up doing several improvements to jextract's error checking and reporting, that I think are useful to keep in the long run.
>> 
>> The changes include:
>> - Using `clang_parseTranslationUnit2` to parse, since that returns an error code, instead of a null translation unit. This also means we can still process diagnostic for the returned translation unit (now returned in a buffer).
>> - Checking the return codes for `clang_parseTranslationUnit2`, `clang_saveTranslationUnit`, and `clang_reparseTranslationUnit` (using a proper enum) and throwing an exception in case they are erroneous.
>> - Catching BadMacroException instead of Throwable in the macro parser. We only care about the former, and catching Throwable could lead to too many exceptions being eaten. Technically, if there is an unknown exception, we could just skip the macro, but this isn't always the right strategy. For example, when re-parsing fails the used translation unit becomes invalid, so we really need to do something else than just skipping the macro in that case (to be addressed in another patch https://bugs.openjdk.java.net/browse/JDK-8240614). Imo it's better to just crash than to skip when we encounter an unknwon exception, so that we're more likely to find issues.
>> - Enabling libclang crash recovery by default. By default the crash recovery was being disabled. This leads to the process exiting with OS exception code. If we enable this however, it will instead try to return an error code from the libclang call, which we can then check for and handle the failure however we want.
>> 
>> Thanks,
>> Jorn
>
> The pull request has been updated with 1 additional commit.

src/jdk.incubator.jextract/share/classes/jdk/internal/clang/LibClang.java line 39:

> 38:         Index_h.clang_toggleCrashRecovery(CRASH_RECOVERY ? 1 : 0);
> 39:         if (DEBUG && !CRASH_RECOVERY) {
> 40:             System.err.println("LibClang crash recovery disabled");

Crash recovery is disabled by default, right? Shouldn't the debug for enabling? (because that is the explicit config behaviour - or else this message will be printed always)

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

PR: https://git.openjdk.java.net/panama-foreign/pull/40