RFR: Filter unsupported constructs in a separate pass [v3]

[Jorn Vernee]

On Fri, 1 Dec 2023 22:32:10 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR moves all the logic to filter unsupported constructs in a dedicated visitor pass, hence removing all the checks from `OutputFactory`. This makes `OutputFactory` a lot simpler, as now this class only has to check for the presence of the `Skip` attribute.
>> 
>> One map in output factory (to keep track of generated structs) has been removed; we can in fact just set the `Skip` attribute on a scoped declaration as soon as we process it, to achieve the same effect. This map was also used in `visitTypedef`, so that we could fetch the correct supertype for the typedef declaration. This logic has now been replaced by making the `JavaName` attribute more expressive, so that we can deal with qualified names.
>> 
>> The other map could not be removed, and has been moved instead inside `NameMangler`. The issue is that we need some way to link a typedef type to the name of the functional interface associated with the typedef. Unfortunately, despite many attempts, I realized that the typedef type does not share anything in common with the typedef declaration; as such, piggy backing on existing attributes is not feasible. So for now, I kept a map, to retain existing behavior. It is possible that the IR might have to be tweaked to accommodate this use case.
>> 
>> Since `NameMangler` now correctly set the functional interface name on all variables whose type is either a function pointer, or a function pointer typedef, the logic in `OutputFactory` can, again, be simplified: now `OutputFactory` only needs to fetch the name attribute set by the mangler (if any).
>
> Maurizio Cimadamore has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove unused import
>  - Drop unused function

src/main/java/org/openjdk/jextract/impl/UnsupportedFilter.java line 69:

> 67:             warn("skipping " + funcTree.name() + " because of unsupported type usage: " +
> 68:                     unsupportedType);
> 69:             Skip.with(funcTree);

Now that the Skip is (only) put on the `funcTree`, I think we could return early from `visitFunction` here, after putting the Skip. (same in some of the other cases)

src/main/java/org/openjdk/jextract/impl/UnsupportedFilter.java line 118:

> 116:         Type.Function func = Utils.getAsFunctionPointer(varTree.type());
> 117:         if (func != null) {
> 118:             checkFunctionTypeSupported(varTree, func);

Shouldn't we do something with the return value here? Now `checkFunctionTypeSupported` is side-effect free I think?

-------------

PR Review Comment: https://git.openjdk.org/jextract/pull/151#discussion_r1413767270
PR Review Comment: https://git.openjdk.org/jextract/pull/151#discussion_r1412635296