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

Jorn Vernee jbvernee at xs4all.nl
Thu Mar 14 00:23:54 UTC 2019 

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.

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