[foreign] RFR 8220544: Jextract; library dependent header filtering

Jorn Vernee jbvernee at xs4all.nl
Fri Mar 15 13:20:31 UTC 2019


Sundararajan Athijegannathan schreef op 2019-03-15 05:37:
> Sorry - it breaks libproc sample. libproc sample was okay in earlier
> round of patch.
> 
> While jextracing libproc.h, there are more warnings of missing symbols
> now and the resulting jar does not work.
> 
> $ java -cp libproc.jar  LibprocMain.java
> LibprocMain.java:13: error: cannot find symbol
>             int numPids = proc_listallpids(Pointer.ofNull(), 0);
>                           ^
>   symbol:   method proc_listallpids(Pointer<Object>,int)
>   location: class LibprocMain
> LibprocMain.java:17: error: cannot find symbol
>             proc_listallpids(pids.elementPointer(), numPids);
>             ^
>   symbol:   method proc_listallpids(Pointer<Integer>,int)
>   location: class LibprocMain
> LibprocMain.java:25: error: cannot find symbol
>                 proc_name(pid, nameBuf, NAME_BUF_MAX);
>                 ^
>   symbol:   method proc_name(int,Pointer<Byte>,int)
>   location: class LibprocMain
> 3 errors
> error: compilation failed
> 
> I am not sure if this dependency analysis/filtering scheme is risk 
> free.

Replied to  this in the other email.

> Few review comments on source changes:
> 
> * ClassLoader.c
> 
>  499     char *buf = malloc(size);
> 
> Null check and throw OOM is missing. Please see elsewhere in the same 
> file:
> 
>     if (body == 0) {
>         JNU_ThrowOutOfMemoryError(env, 0);
>         return 0;
>     }

Sounds good, but this code will probably be dropped.

> * JextractToolRunner.java
> 
>  250                 return null;
> 
> 
> Why is that change? Probably I missed explanation in another email on
> this thread. A comment there would be useful.

I'll add a comment; A lot of the tests are loading a class and then 
checking if it's non-null to see if it exists. I wanted to check if a 
class was non-existent by checking if the returned class was null, but 
in that case the class loader code also needs to return null if a class 
is not found.

> * test/jdk/com/sun/tools/jextract/test8219442/point.h
> 
> Do we really need that non-standard #pragma once? Can we use #ifndef
> here? Given that it is just a test ...
> Few other tests use the same pragma as well. Same comment.

I'll do the standard header guard.

> * DependencyFilter and ElementFilter could have one liner comment on
> the purpose.

Ok.

> * RootSet class
> - Two mutable collection fields are public final. Can we expose just
> iterator for these for caller?

Whoops, those should be private actually.

Thanks,
Jorn

> -Sundar
> 
> On 15/03/19, 6:56 AM, Jorn Vernee wrote:
>> Here's the updated webrev: 
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.05/
>> 
>> This includes your patch as well.
>> 
>> Thanks,
>> Jorn
>> 
>> Jorn Vernee schreef op 2019-03-15 01:03:
>>> Oh... I thought findEntry was added as part of Panama. I guess the
>>> current difference in behavior between platforms exists in jdk/jdk as
>>> well then.
>>> 
>>> Your patch works as well, I just thought that the VM code should try
>>> to offer up uniform behavior, i.e. findEntry should always do a
>>> non-recursive lookup on every platform. But, If the function is
>>> pre-existing we should probably avoid changing the behavior.
>>> 
>>> I've tried with your patch as well, but 5 tests are failing with
>>> symbol lookup errors. It seems to be a problem with the default
>>> library... i.e. we should not do filtering when using the default
>>> library because the library name will always be "<default>". I'm
>>> testing a fix.
>>> 
>>> Jorn
>>> 
>>> Maurizio Cimadamore schreef op 2019-03-15 00:27:
>>>> Not super sure about this approach - findEntry was there before,
>>>> aren't we changing the semantics of that function with your changes?
>>>> 
>>>> This is in jdk/jdk (not Panama)
>>>> 
>>>> http://hg.openjdk.java.net/jdk/jdk/file/8a66c92526cb/src/java.base/share/native/libjava/ClassLoader.c#l448 
>>>> That will most surely cause JNI link differences here
>>>> 
>>>> http://hg.openjdk.java.net/jdk/jdk/file/8a66c92526cb/src/java.base/share/classes/java/lang/ClassLoader.java#l2690 
>>>> That was the reason why I put the changes inside the lookup() 
>>>> method.
>>>> 
>>>> We could make a new lookup method for it, but I'd like first to
>>>> understand why you think this filtering cannot be done in Java 
>>>> (since
>>>> the functionality I'm using is supported on all platforms?)
>>>> 
>>>> Maurizio
>>>> 
>>>> On 14/03/2019 23:00, Jorn Vernee wrote:
>>>>> FWIW, I've implemented the changes to findEntry based on your 
>>>>> patch: 
>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.04/
>>>>> 
>>>>> Jorn
>>>>> 
>>>>> Jorn Vernee schreef op 2019-03-14 20:53:
>>>>>> Sounds good!
>>>>>> 
>>>>>> As for the implementation; since the filtering requirement is 
>>>>>> platform
>>>>>> specific, it seems better to implement findEntry as a platform
>>>>>> specific function in 
>>>>>> java.base/<platform>/native/libjava/jni_util_md.c
>>>>>> (e.g. [1]) ?
>>>>>> 
>>>>>> We already have a function there for findEntryInProcess. We could 
>>>>>> add
>>>>>> one for findEntry as well, and then filter on unix.
>>>>>> 
>>>>>> Jorn
>>>>>> 
>>>>>> [1] :
>>>>>> http://hg.openjdk.java.net/panama/dev/file/b3831a541894/src/java.base/unix/native/libjava/jni_util_md.c 
>>>>>> Maurizio Cimadamore schreef op 2019-03-14 20:37:
>>>>>>> This is my take on the issue (see attached patch).
>>>>>>> 
>>>>>>> Basically check path of library symbol against library path 
>>>>>>> itself,
>>>>>>> and if they don't match do not return the symbol via 
>>>>>>> Library::lookup.
>>>>>>> This should provide stable semantics across platforms.
>>>>>>> 
>>>>>>> Maurizio
>>>>>>> 
>>>>>>> On 14/03/2019 19:10, Maurizio Cimadamore wrote:
>>>>>>>> We do have soe support in the VM os class:
>>>>>>>> 
>>>>>>>> // Locate DLL/DSO. On success, full path of the library is 
>>>>>>>> copied to
>>>>>>>>   // buf, and offset is optionally set to be the distance 
>>>>>>>> between addr
>>>>>>>>   // and the library's base address. On failure, buf[0] is set 
>>>>>>>> to '\0'
>>>>>>>>   // and offset is set to -1 (if offset is non-NULL).
>>>>>>>>   static bool dll_address_to_library_name(address addr, char* 
>>>>>>>> buf,
>>>>>>>>                                           int buflen, int* 
>>>>>>>> offset);
>>>>>>>> 
>>>>>>>> 
>>>>>>>> trying to play with it and see what happens. Seems implemented 
>>>>>>>> for all OSs.
>>>>>>>> 
>>>>>>>> Maurizio
>>>>>>>> 
>>>>>>>> On 14/03/2019 18:58, Jorn Vernee wrote:
>>>>>>>>> Here's the Windows prototype I had for iterating the symbols in 
>>>>>>>>> a library: 
>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/itersyms/webrev.00 
>>>>>>>>> Jorn
>>>>>>>>> 
>>>>>>>>> Jorn Vernee schreef op 2019-03-14 19:50:
>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 19:32:
>>>>>>>>>>> So, I think that if we don't address this in some way, this 
>>>>>>>>>>> work is
>>>>>>>>>>> not very effective on Linux/Mac, do yo agree?
>>>>>>>>>> 
>>>>>>>>>> Yes, this is unfortunate...
>>>>>>>>>> 
>>>>>>>>>>> As to how to address it, since there doesn't seem to be a way 
>>>>>>>>>>> to
>>>>>>>>>>> perform a 'local' library search, what if we augmented the 
>>>>>>>>>>> Symbol
>>>>>>>>>>> interface to also return a Path (of the shared library the 
>>>>>>>>>>> symbol
>>>>>>>>>>> belongs to) ?
>>>>>>>>>>> 
>>>>>>>>>>> With that info we should be able to do some more filtering?
>>>>>>>>>>> 
>>>>>>>>>>> To get the path, on *NIX we can use dladdr. Not sure if Win 
>>>>>>>>>>> has
>>>>>>>>>>> something similar.
>>>>>>>>>> 
>>>>>>>>>> I ran into dladdr as well, but that seems to be a glibc 
>>>>>>>>>> extension? On
>>>>>>>>>> Windows we have GetModuleFileName [1], so at least that's 
>>>>>>>>>> covered.
>>>>>>>>>> 
>>>>>>>>>> But, it might be preferable to try and implement the symbol 
>>>>>>>>>> table
>>>>>>>>>> crawl on the different operating systems, since that's more 
>>>>>>>>>> straight
>>>>>>>>>> towards what we want to achieve. I mean, if we're going the
>>>>>>>>>> non-portable route any ways... At least on Linux this seems to 
>>>>>>>>>> be
>>>>>>>>>> possible as well [2] I want to say that this should be 
>>>>>>>>>> possible to do
>>>>>>>>>> on most operating systems, as long as we can load the library 
>>>>>>>>>> file
>>>>>>>>>> directly into memory, and know the file format.
>>>>>>>>>> 
>>>>>>>>>> If we go that route I would need some help with the other 
>>>>>>>>>> platforms of course :)
>>>>>>>>>> 
>>>>>>>>>> Jorn
>>>>>>>>>> 
>>>>>>>>>> [1] :
>>>>>>>>>> https://docs.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-getmodulefilenamea 
>>>>>>>>>> [2] :
>>>>>>>>>> https://stackoverflow.com/questions/26960377/how-to-programmatically-list-elf-shared-library-symbols
>>>>>>>>>>> Maurizio
>>>>>>>>>>> 
>>>>>>>>>>> On 14/03/2019 17:41, Maurizio Cimadamore wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> On 14/03/2019 17:36, Jorn Vernee wrote:
>>>>>>>>>>>>> I think you got it
>>>>>>>>>>>>> 
>>>>>>>>>>>>> With dlsym it seems to be recursive: [1] "The search 
>>>>>>>>>>>>> performed by dlsym() is breadth first through the 
>>>>>>>>>>>>> dependency tree of these libraries."
>>>>>>>>>>>>> 
>>>>>>>>>>>>> But, GetProcAdress on Windows [2] looks at the export table 
>>>>>>>>>>>>> of the library you give it, so AFAICT this is 
>>>>>>>>>>>>> non-recursive.
>>>>>>>>>>>> 
>>>>>>>>>>>> Yep - found the same, you beat me to it :-)
>>>>>>>>>>>> 
>>>>>>>>>>>> Maurizio
>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>> 
>>>>>>>>>>>>> [1] : https://linux.die.net/man/3/dlsym
>>>>>>>>>>>>> [2] : 
>>>>>>>>>>>>> https://docs.microsoft.com/en-us/windows/desktop/api/libloaderapi/nf-libloaderapi-getprocaddress
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 18:25:
>>>>>>>>>>>>>> Nope - with this:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> $ cat foo.h
>>>>>>>>>>>>>> #include <string.h>
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> void foo(int f);
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> $ cat foo.c
>>>>>>>>>>>>>> #include "foo.h"
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> void foo(int f) { }
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> then,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> $  gcc -shared -o libfoo.so foo.c
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> $ jextract -L . -l foo -d classes foo.h
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> And then
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> classes
>>>>>>>>>>>>>> ├── clang_support
>>>>>>>>>>>>>> │   ├── stddef.class
>>>>>>>>>>>>>> │   ├── stddef_h.class
>>>>>>>>>>>>>> │   └── stddef$size_t.class
>>>>>>>>>>>>>> ├── foo.class
>>>>>>>>>>>>>> ├── foo_h.class
>>>>>>>>>>>>>> ├── META-INF
>>>>>>>>>>>>>> │   └── jextract.properties
>>>>>>>>>>>>>> └── usr
>>>>>>>>>>>>>>     └── include
>>>>>>>>>>>>>>         ├── string.class
>>>>>>>>>>>>>>         ├── string_h.class
>>>>>>>>>>>>>>         ├── xlocale.class
>>>>>>>>>>>>>>         ├── xlocale_h.class
>>>>>>>>>>>>>>         ├── xlocale.class
>>>>>>>>>>>>>>         ├── xlocale.class
>>>>>>>>>>>>>>         └── xlocale.class
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 4 directories, 13 files
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I wonder if some of the library lookup functions we have 
>>>>>>>>>>>>>> on linux
>>>>>>>>>>>>>> (dlopen and co.) are doing a 'recursive' search into 
>>>>>>>>>>>>>> dependent libs as
>>>>>>>>>>>>>> well, which might be disabled on Win.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On 14/03/2019 16:38, Maurizio Cimadamore wrote:
>>>>>>>>>>>>>>> Ah ok
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> will give that another try
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On 14/03/2019 16:16, Jorn Vernee wrote:
>>>>>>>>>>>>>>>> Sorry, that's with the v3 patch 
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.03/ 
>>>>>>>>>>>>>>>> I reverted back after the concerns raised about changing 
>>>>>>>>>>>>>>>> the implementation of TypedefTree::def You're right in 
>>>>>>>>>>>>>>>> that this incorrectly includes some trees.
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Sorry,
>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 17:08:
>>>>>>>>>>>>>>>>> This produces test.class and test_h.class as expected, 
>>>>>>>>>>>>>>>>> but nothing else...
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Used command is: `jextract -L . -l test test.h`
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> You would get stuff from string.h when the -l is 
>>>>>>>>>>>>>>>>> omitted.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 17:00:
>>>>>>>>>>>>>>>>>> I think you can reproduce simply by having an header 
>>>>>>>>>>>>>>>>>> like this:
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> #include <string.h>
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> void foo(int i);
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> if you extract that you would expect one symbol, but 
>>>>>>>>>>>>>>>>>> in reality you
>>>>>>>>>>>>>>>>>> get also all the stuff from string.h.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> On 14/03/2019 15:49, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>> The Windows jextract artifact for the OpenGL sample 
>>>>>>>>>>>>>>>>>>> is a different beast, because it relies on the 
>>>>>>>>>>>>>>>>>>> standard Windows OpenGL libraries. So, seeing some 
>>>>>>>>>>>>>>>>>>> system headers is to be expected :) In the case of 
>>>>>>>>>>>>>>>>>>> python I'm also seeing some system headers, because 
>>>>>>>>>>>>>>>>>>> python depends on a hand full of corecrt types, so 
>>>>>>>>>>>>>>>>>>> that's also correct. But, neither one seems to be 
>>>>>>>>>>>>>>>>>>> referencing imaxabs, so I'm not sure what's going 
>>>>>>>>>>>>>>>>>>> on...
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> I'll try to see if I can find a repro example on 
>>>>>>>>>>>>>>>>>>> Windows to see about a fix. I'm not sure I see how 
>>>>>>>>>>>>>>>>>>> the SymbolFilter could get to the VM libraries? 
>>>>>>>>>>>>>>>>>>> Though I remember seeing a similar case in the 
>>>>>>>>>>>>>>>>>>> past...
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 15:55:
>>>>>>>>>>>>>>>>>>>> I think I got at the bottom of this. I tried 
>>>>>>>>>>>>>>>>>>>> debugging an extraction
>>>>>>>>>>>>>>>>>>>> of opengl, and it eventually ends up adding 
>>>>>>>>>>>>>>>>>>>> inttypes.h to the rootset
>>>>>>>>>>>>>>>>>>>> because of this symbol:
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> imaxabs
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> The library filter doesn't work here, because this 
>>>>>>>>>>>>>>>>>>>> is a function
>>>>>>>>>>>>>>>>>>>> defined in the set of default libraries that comes 
>>>>>>>>>>>>>>>>>>>> with the VM, so
>>>>>>>>>>>>>>>>>>>> jextract 'thinks' this symbol is genuinely part of 
>>>>>>>>>>>>>>>>>>>> one of the
>>>>>>>>>>>>>>>>>>>> libraries specified in the command line - except is 
>>>>>>>>>>>>>>>>>>>> not.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> I tried an hacky fix which checks for absence of 
>>>>>>>>>>>>>>>>>>>> symbol in
>>>>>>>>>>>>>>>>>>>> Libraries.getDefaultLibrary - and that seems to give 
>>>>>>>>>>>>>>>>>>>> the expected
>>>>>>>>>>>>>>>>>>>> results - but I leave this to your consideration.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> On 14/03/2019 14:27, Maurizio Cimadamore wrote:
>>>>>>>>>>>>>>>>>>>>> Hi Jorn,
>>>>>>>>>>>>>>>>>>>>> I gave this a try with our OpenGL and Python 
>>>>>>>>>>>>>>>>>>>>> examples. The extraction process works fine, and 
>>>>>>>>>>>>>>>>>>>>> the generated jars are a bit smaller - but I still 
>>>>>>>>>>>>>>>>>>>>> see a lot of system-related headers. Python has 
>>>>>>>>>>>>>>>>>>>>> still lots of them, OpenGL less so, but even in the 
>>>>>>>>>>>>>>>>>>>>> GL case, I think it's interesting, because I can't 
>>>>>>>>>>>>>>>>>>>>> see references to types mentioned in these headers 
>>>>>>>>>>>>>>>>>>>>> (nor in the resulting classfiles). Do you know, on 
>>>>>>>>>>>>>>>>>>>>> top of your head, as to why this could be 
>>>>>>>>>>>>>>>>>>>>> happening?
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> On 14/03/2019 12:45, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>> Updated webrev: 
>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.04/ 
>>>>>>>>>>>>>>>>>>>>>> Changed Treemaker implementation to recurse to the 
>>>>>>>>>>>>>>>>>>>>>> 'root' type when creating TypdefTree::def, and 
>>>>>>>>>>>>>>>>>>>>>> using that in DependencyFinder.
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 12:49:
>>>>>>>>>>>>>>>>>>>>>>> Oh ok, sorry. I'll try that out.
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 12:43:
>>>>>>>>>>>>>>>>>>>>>>>> On 14/03/2019 11:40, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> I see what you mean.
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Well, `struct BAR` has a definition cursor, but 
>>>>>>>>>>>>>>>>>>>>>>>>> `struct BAR*` does not have a 
>>>>>>>>>>>>>>>>>>>>>>>>> definition/declaration, it's a type that's 
>>>>>>>>>>>>>>>>>>>>>>>>> automatically derived from `struct BAR`. So for 
>>>>>>>>>>>>>>>>>>>>>>>>> `struct BAR*` there is no good value for 
>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def.
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> I know that there's no good value for struct 
>>>>>>>>>>>>>>>>>>>>>>>> BAR* - I was wondering if
>>>>>>>>>>>>>>>>>>>>>>>> the parser should detect the pointer, and 
>>>>>>>>>>>>>>>>>>>>>>>> recurse down, then store
>>>>>>>>>>>>>>>>>>>>>>>> 'struct BAR' into 'def'.
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def seems to be a helper method 
>>>>>>>>>>>>>>>>>>>>>>>>> mostly targeted at inline type definitions 
>>>>>>>>>>>>>>>>>>>>>>>>> (looking at TypedefHandler [1]).
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> I think for my use case using 
>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree.type().canonicalType() is good 
>>>>>>>>>>>>>>>>>>>>>>>>> enough, but I guess we could query that once in 
>>>>>>>>>>>>>>>>>>>>>>>>> the parser and then store it in a field in 
>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree?
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> [1] : 
>>>>>>>>>>>>>>>>>>>>>>>>> http://hg.openjdk.java.net/panama/dev/file/43ed53f40957/src/jdk.jextract/share/classes/com/sun/tools/jextract/TypedefHandler.java#l107
>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 
>>>>>>>>>>>>>>>>>>>>>>>>> 11:57:
>>>>>>>>>>>>>>>>>>>>>>>>>> On 14/03/2019 00:23, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>> Sorry, bad example. The actual failure case 
>>>>>>>>>>>>>>>>>>>>>>>>>>> is:
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>      struct BAR { int x; }
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>      typedef struct BAR* FOO;
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> The TreeMaker code looks at the canonical 
>>>>>>>>>>>>>>>>>>>>>>>>>>> type of the typedef `struct BAR*` which is 
>>>>>>>>>>>>>>>>>>>>>>>>>>> neither a type definition or declaration, so 
>>>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def is empty. But, in the case 
>>>>>>>>>>>>>>>>>>>>>>>>>>> of dependecy analysis, we still want to 
>>>>>>>>>>>>>>>>>>>>>>>>>>> inspect that type, get the pointee type and 
>>>>>>>>>>>>>>>>>>>>>>>>>>> add it to the root set.
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> Right - and I think what I'm claiming is that 
>>>>>>>>>>>>>>>>>>>>>>>>>> this is probably some
>>>>>>>>>>>>>>>>>>>>>>>>>> kind of a parser bug? That is, if Typedef::def 
>>>>>>>>>>>>>>>>>>>>>>>>>> (we should probably
>>>>>>>>>>>>>>>>>>>>>>>>>> find another name, 'def' is not good enough 
>>>>>>>>>>>>>>>>>>>>>>>>>> IMHO) is set correctly for
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> typedef struct BAR FOO;
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> it should also be set for
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> typedef struct BAR* FOO;
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> That is, I claim that from the typedef tree 
>>>>>>>>>>>>>>>>>>>>>>>>>> you should always see a
>>>>>>>>>>>>>>>>>>>>>>>>>> referenced type declaration w/o using the 
>>>>>>>>>>>>>>>>>>>>>>>>>> clang API.
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-14 01:14:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-14 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 01:04:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> So, if I understand correctly, the issue is 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that if you have:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> typedef struct { int x; } FOO;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you want the struct to be added to the root 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> set. Which make sense.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Now, with respect to getUnderlyingType - 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm a bit at odds; it seems
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to me that the jextract tree should have 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the right info, under
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def, at least that is set by 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the parser for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> declarations/definitions.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> That exact case is covered by 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def, but it doesn't cover a
>>>>>>>>>>>>>>>>>>>>>>>>>>>> case like this:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>     struct BAR { int x; }
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>     typedef struct BAR FOO;
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> i.e. TypedefTree::def is only set when the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> typedef contains an inline
>>>>>>>>>>>>>>>>>>>>>>>>>>>> definition or a prototype. But, if the type 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> is just referenced,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> TypedefTree::def is an empty Optional.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Wouldn't visiting TypedefTree::def (if 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> present) be enough (and more in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sync with the rest of jextract) ? After 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all, that's the kind of thing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you wanna be sure not to miss by the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dependency analysis -
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> getUnderlyingType seems a tad too noisy?
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is what I had before, but it fell short 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> in some cases.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In other words, let's say that there's a 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> typedef in an header of a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> library you want to jextract which is 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> related, via a chain of other 42
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> typedefs to something in some obscure 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> system library. Are you sure you
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> want the resulting artifact to have all the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 42 intermediate typedefs?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> After all, the library you wanna extract 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> has just defined 'its
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> version' of the name for that particular 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> construct... so it's very
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> likely that users of that library will 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> prefer that name to the other
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 42 ones?
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 42 intermediate typedefs seems a bit 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> extreme...
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> But, I think you have a point. Putting the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> typedef and the canonical
>>>>>>>>>>>>>>>>>>>>>>>>>>>> type into the root set is probably enough.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 23:31, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Update webrev: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.02/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This fixes an issue where typedef 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> underlying types were not being added to 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the root set.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It turns out I was using the wrong API to 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> get the underlying type, and the needed 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> API didn't have a binding yet :)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> The changes with respect to the previous 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> webrev;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * added 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cursor.getTypedefDeclUnderlyingType() 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> binding
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * added TypdefTree.underlyingType() 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> method
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * changed DependencyFinder to use this 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> method instead of typeDefinition()
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * added test case for the failure case 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (also simplified test class to only run 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> jextract once and reuse the result)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 2019-03-13 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 22:43:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 21:36, Jorn Vernee wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Good questions. When no filter patterns 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are specified that is the case
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> everything is outputted, but when 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filter patterns are specified
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers are only added to the root set 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> if they contain a symbol that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> passes the filter.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> FWIW, basically headers with symbols 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that pass both types of filter are added 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to the root set. There's 1.) Filtereing 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> based on whether a symbol is found in a 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> library, and 2.) based on the used 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filter patterns (--include-symbols & 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> --exclude-symbols). I thought it would 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be better to exclude a header from the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> root set if all the symbols in it were 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filtered out by the filter patterns.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yep - I think that sounds sensible
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 22:30:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio Cimadamore schreef op 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2019-03-13 22:08:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jorn,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> this looks very good indeed. I'd like 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to play with it a bit before we
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> go ahead with this, if that's ok.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please do!
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Also, one question: if the user does 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not specify "-l" what happens? I
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> guess the root set will then contain 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all the headers and we just get
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> back the full output (which would be 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ok) ?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Good questions. When no filter patterns 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> are specified that is the case
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> everything is outputted, but when 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filter patterns are specified
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers are only added to the root set 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> if they contain a symbol that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> passes the filter.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maybe we should just fall back to 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 'include-everything' when no "-l"
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> options are specified, regardless of 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filter?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maurizio
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 13/03/2019 18:06, Jorn Vernee 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Updated webrev: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.01/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Fixed the bug, also added tests & ran 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> all the examples again to try and 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> find other bugs (nothing found).
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 17:50:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Discovered a bug in this after 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> posting :(
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> DependencyFinder also needs to look 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for pointee and array element type
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> cursors (which it currently 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> doesn't). I've tested a working fix, 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> but
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will also add some tests for the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> particular cases. Updated webrev
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> coming soon™
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn Vernee schreef op 2019-03-13 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 17:21:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> After some discussion:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://mail.openjdk.java.net/pipermail/panama-dev/2019-March/004828.html 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've implemented the discussed 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> approach to header & declaration 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filtering.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Bug: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220544 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Webrev: 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8220544/webrev.00/ 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> There are a few new implementation 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> classes:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * PattertnFilter; a helper class 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that encapsulates the Pattern based
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> filtering code previously found in 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> SymbolFilter. This code had to be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> reused.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * Filters; a Context 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sub-component holding 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PatternFilters for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> different types of declarations.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * DependencyFinder; finds 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dependencies of the root set of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> declarations found in the root 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> headers.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * ElementFilter; filters out 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> non-library symbol elements based 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> whether they appear in a root 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> header or are required by something
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> therein.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>   * RootSet; a helper class for 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> managing root headers + required 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> element Trees.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I also encountered some crashes 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> when debug printing Trees, which 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> fixed/worked around accordingly.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> FWIW, I tried this out with the 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Windows registry API and it's 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> working
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> very nicely! Going from well over 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 100 headers to under 10. Some room
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for improvement still exists; some 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> empty headers and static forwarders
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> could still be omitted, but imho 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that should be handled by a 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> separate
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> patch.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jorn


More information about the panama-dev mailing list