RFR: 7903600: Add more tests for unsupported types
Jorn Vernee
jvernee at openjdk.org
Mon Dec 4 20:52:29 UTC 2023
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 `Sytstem.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 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 check 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.
test/lib/testlib/JextractToolRunner.java line 149:
> 147: StringWriter writer = new StringWriter();
> 148: PrintWriter pw = new PrintWriter(writer);
> 149: String[] args = new String[options.length + 1];
Also dead code.
test/testng/org/openjdk/jextract/test/toolprovider/Test8245767.java line 50:
> 48: Class<?> fooCls = loader.loadClass("Foo");
> 49: assertNotNull(fooCls);
> 50:
This is the problematic test case that I believe to be testing for behavior that we do not want.
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/152#discussion_r1414470925
PR Review Comment: https://git.openjdk.org/jextract/pull/152#discussion_r1414471689
More information about the jextract-dev
mailing list