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

[Athijegannathan Sundararajan]

On Thu, 26 Mar 2020 10:19:54 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
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Change crash recovery debug message to print enabled/disabled

Looks good!

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

Marked as reviewed by sundar (Committer).

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