[foreign] RFC: Additional jextract filtering

[Jorn Vernee]

Maurizio Cimadamore schreef op 2019-03-11 14:24:
> Hi Jorn,
> didn't look at the patch, this sounds like an ambitious piece of work
> (and one we have been needing for quite some time); I'll start from
> some high level comments:
> 
> * how do you determine whether a macro belongs or not to a root set?
> It's a bit like structs but worse, as structs actually do show up in
> functions, but macros do not (probably enums are tricky too?)

Exactly, macros don't show up in functions, so by default they are all 
included.

Enums are tricky as well, like you say, we can have the following 
situation:

     enum option { x, y, z }

     void func(int options);

Where the enum type does not appear in the function signature.

I think it's best to keep the default on including everything for that 
reason. The header file should tell us what's required to use the 
library. It becomes tricky when we only want to generate stuff for a 
couple of library symbols, since then you might end up with a lot of 
junk that's not needed, since the header contains the entire library. In 
that case you could switch to using the REQUIRED preset, and use 
additional --include options to include anything that is not in the root 
set, but is still needed.

> * I think the option scheme seems to convoluted. I get the desire to
> expose something uniform, and that might indeed a good choice for the
> jextract internal. But my claim is that the user should never have to
> do "--include xyz-REQUIRED", to obtain something minimal and sensible
> - that should just be the default. It seems like your options always
> 'play it safe' e.g. they stay within the limit of generating something
> that's actually bindable, which is good.

Well, the header file tells us what's sensible, so I believe 
no-filtering should be the default. We could also turn on the REQUIRED 
preset once other include or exclude patterns start being passed, but 
that seems a bit magic.

The options are quite convoluted, but I think it beats having 2 options 
for each element type, and having to add more options if a new element 
type comes along. Having a separate option for setting the filter 
presets could be an idea.

> It seems to me that figuring out the right way to expose/surface these
> options in hard, and I don't want that part of discussion to block the
> other good work you have done here. Is there a way to separate the two
> - e.g. add the root calculation w/o altering the options for now (or
> making sure that the options we have now do not end up violating the
> root set computation) ?

AFAIK The options we have now can never violate the root set, since they 
only work on library symbols and macros, which nothing else should 
depend on.

I can separate the parts of the patch a little bit into; Filter refactor 
+ root set compute, and then leave the option changes out of it. But 
those 2 alone do not affect the filtering, since the root set is only 
used when filtering non-symbol/macro elements.

Jorn

> Maurizio
> 
> 
> 
> On 11/03/2019 13:04, Jorn Vernee wrote:
>> Hi,
>> 
>> I've been looking into adding more filtering capabilities to jextract 
>> over the past few days.
>> 
>> I have come up with the following proposal; Replace --include-symbols 
>> and --exclude-symbols with 2 new options, --include and --exclude. 
>> These options take as an argument a filter expression, which looks 
>> like SELECTOR:PATTERN or, for includes only, SELECTOR-PRESET, where 
>> SELECTOR is one of [symbol, macro, struct, typedef, enum], and PRESET 
>> is one of [ALL, REQUIRED]. PATTERN is a regex pattern. The selector 
>> lets us select the type of element a pattern applies to, and the 
>> preset is a default for what to do when there are no explicit includes 
>> specified for that specific selector.
>> 
>> The earlier mentioned problem with allowing filtering of things 
>> besides symbols and macros (which is what the current implementation 
>> supports), is that we might end up excluding something that is 
>> actually required. So, to make sure that doesn't happen, we first 
>> filter a set of 'root' declarations, which are library symbols (global 
>> vars and functions) and macros, then from those compute a minimal set 
>> of dependencies, and then use that in a subsequent round of filtering 
>> for structs, typedefs and enums to make sure nothing that is required 
>> gets filtered out. This minimal dependency set can also be used to 
>> only include what's absolutely needed, by using the REQUIRED filter 
>> preset.
>> 
>> To give an example; If we have a header like so:
>> 
>> ```
>> struct Foo {
>>     int x;
>> };
>> 
>> struct Bar {
>>     int x;
>> };
>> 
>> void func(struct Foo foo);
>> ```
>> 
>> We can use the option `--include struct-REQUIRED`, and `Bar` will be 
>> omitted from the output, since it is not used directly by `func`. 
>> However, if we also use the option `--exclude struct:Foo` the output 
>> won't change, since the struct Foo is required by the function func. 
>> But, if we instead use `--exclude symbol:func` in the options, we get 
>> no output at all, because `func` is filtered, and then none of the 
>> structs are required, so they get filtered as well.
>> 
>> I have implemented a prototype of this proposal here: 
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/filters/webrev.00/
>> 
>> If the proposal sounds good I can add more tests and make an RFR.
>> 
>> Thanks,
>> Jorn
>> 
>> 
>>