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

[Sundararajan Athijegannathan]

Looks good.

-Sundar

On 07/05/19, 2:45 AM, Henry Jen wrote:
> 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/