RFR: 7903600: Add more tests for unsupported types

[Maurizio Cimadamore]

On Mon, 4 Dec 2023 20:42:16 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Add more tests for unsupported types. See the new TestUnsupportedTypes.java
> 
> While working on this, I also noticed several cases of dead code: `FunctionalInterfaceScanner` is completely unused, and `InMemoryJavaCompiler::jfoFromByteArray` and `InMemoryJavaCompiler::jfoFromString` (and its caller in `OutputFactory`) are unused as well. I've removed them in this PR.
> 
> Changes in this PR:
> - I'm passing in the error stream PrintWriter that we create in the test all the way down to UnsupportedFilter, so that it can write to it instead of `System.err`. Then we can check what was written in the test. 
> - I've added warning print outs for the different reasons why we are skipping declarations. Some of these were missing.
> - I've removed a check for ValueLayouts that are larger than 8 bytes in `visitVariable`. If the type of a variable is an unsupported primitive, we already filter it out earlier on in the method, so there's no need to check again.
> - I've added a missing `Skip.with` in `visitVariable`, so that we will filter out (global) variables with unsupported function types.
> - I've added skipping of typedefs when they have an unsupported type or a type that does not have a layout. For the latter, I've also had to remove a test case that checked if we generated a typedef class for an undefined struct type. Since the struct is undefined, I believe we should not generate a typedef binding class for that case.
> 
> The new test covers all the unsupported type cases that are handled by TestUnsupportedTypes.

Marked as reviewed by mcimadamore (Committer).

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

> 114:         if (layout == null) {
> 115:             //no layout - skip
> 116:             warnSkip(name, "can not compute MemoryLayout");

I suggest "does not have a valid memory layout/descriptor"

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

> 190:         String unsupportedType = firstUnsupportedType(func);
> 191:         if (unsupportedType != null) {
> 192:             warnUnsupportedType(nameOfSkipped, unsupportedType);

Should we consolidate this with the other warnings? At the end of the day, we're skipping here because of an unsupported type, so (for consistency) I'd prefer to see `warnSkip` being used, where "unsupported type" appears in the "cause"

test/testng/org/openjdk/jextract/test/toolprovider/unsupported/TestUnsupportedTypes.java line 32:

> 30: import java.nio.file.Path;
> 31: 
> 32: public class TestUnsupportedTypes extends JextractToolRunner {

Nice and comprehensive test - good to have everything in one place!

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

PR Review: https://git.openjdk.org/jextract/pull/152#pullrequestreview-1764663559
PR Review Comment: https://git.openjdk.org/jextract/pull/152#discussion_r1415306901
PR Review Comment: https://git.openjdk.org/jextract/pull/152#discussion_r1415305683
PR Review Comment: https://git.openjdk.org/jextract/pull/152#discussion_r1415308840