[foreign] RFR 8223413: Improve missing symbols handling in jextract

[Henry Jen]

Bug[1] created, and official webrev[2] review request.

[1] https://bugs.openjdk.java.net/browse/JDK-8223413
[2] http://cr.openjdk.java.net/~henryjen/panama/8223413/0/webrev/

Cheers,
Henry

> On May 3, 2019, at 1:34 PM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> If I understand what you are saying, you just want to make explicit the fact that if -l has been set, _some_ library will always be set, either explicitly (via -L) or implicitly (inferred from java.library.path).
> 
> If so, this seems like a good change.
> 
> Maurizio
> 
> On 03/05/2019 18:12, Henry Jen wrote:
>> The current implementation behave correctly as expected, but reading LibraryLookupFilter feels wrong. It depends on the fact that linkCheckPaths is initialized to java.library.path when -l is specified but not -L in Main.java.
>> 
>> At least we can do is to add following change,
>> 
>> --- a/src/jdk.jextract/share/classes/com/sun/tools/jextract/LibraryLookupFilter.java
>> +++ b/src/jdk.jextract/share/classes/com/sun/tools/jextract/LibraryLookupFilter.java
>> @@ -86,8 +86,9 @@
>>      }
>> 
>>      private void initSymChecker(List<String> linkCheckPaths) {
>> -        if (!libraryNames.isEmpty() && !linkCheckPaths.isEmpty()) {
>> +        if (!libraryNames.isEmpty()) {
>>              try {
>> +                assert !linkCheckPaths.isEmpty();
>>                  Library[] libs = loadLibraries(MethodHandles.lookup(),
>>                          linkCheckPaths.toArray(new String[0]),
>>                          libraryNames.toArray(new String[0]));
>> 
>> Cheers,
>> Henry
>> 
>>> On May 3, 2019, at 10:02 AM, Henry Jen <henry.jen at oracle.com> wrote:
>>> 
>>> Correct.
>>> 
>>> Cheers,
>>> Henry
>>> 
>>> 
>>>> On May 3, 2019, at 10:00 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
>>>> 
>>>> Seems like an useful follow up. Just to make sure I understand, if -l is specified, we get same behavior as before. If -l is NOT specified, then the behavior would differ (as we now do the check), and therefore we need the 'ignore' option explicitly, if we want to suppress logging. Right?
>>>> 
>>>> Maurizio
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On 03/05/2019 17:43, Henry Jen wrote:
>>>>> Hi,
>>>>> 
>>>>> Please review a webrev[1] that add the missing —missing-symbols warn support, and turn on symbol checking against default library by default.
>>>>> 
>>>>> This is kind of a follow up to JDK-8223247, as that simply assume we are using the default libraries within JVM if no -l option is provided. This webrev now will
>>>>> 
>>>>> - Same behavior as before if both -l and -L are provided.
>>>>> - Symbol check is turned on always. If there is no -l provided, jextract will check symbols against the default library.
>>>>> - Default is to issue warnings without -l, exclude with explicit -l. This is mostly backward compatible as it doesn’t change generated code/classes, but show warnings to inform user about potential missing libraries.
>>>>> 
>>>>> To be 100% compatible with before, use '—missing-symbols ignore’
>>>>> 
>>>>> Thoughts?
>>>>> 
>>>>> Cheers,
>>>>> Henry
>>>>> 
>>>>> [1] http://cr.openjdk.java.net/~henryjen/panama/missingSymbols/webrev/