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

[Henry Jen]

Thanks, pushed. I also sneak in a fix for intermmitent test failure for 8223105 on Windows.

Cheers,
Henry


diff -r 34449b3f3b3d test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java
--- a/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java  Wed May 08 18:40:07 2019 +0530
+++ b/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java  Wed May 08 08:18:36 2019 -0700
@@ -33,7 +33,7 @@
  * @requires os.family == "windows"
  * @library ..
  * @run driver JtregJextract -C -DADD -t test.jextract.asmsymbol -- libAsmSymbol.h
- * @run testng Test8223105WinA
+ * @run testng/othervm Test8223105WinA
  */
 public class Test8223105WinA {
     static final libAsmSymbol_h libAsmSymbol;
diff -r 34449b3f3b3d test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java
--- a/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java  Wed May 08 18:40:07 2019 +0530
+++ b/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java  Wed May 08 08:18:36 2019 -0700
@@ -33,7 +33,7 @@
  * @requires os.family == "windows"
  * @library ..
  * @run driver JtregJextract -t test.jextract.asmsymbol -- libAsmSymbol.h
- * @run testng Test8223105WinB
+ * @run testng/othervm Test8223105WinB
  */
 public class Test8223105WinB {
     static final libAsmSymbol_h libAsmSymbol;


> On May 8, 2019, at 2:58 AM, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
> 
> 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/