RFR: 7903783: Jextract should tolerate more CPP constructs when running with xc++
This PR fixes a bunch of issues with trying to run jextract in CPP mode, using a `compile_flags.txt` file containing the `-xc++` flag. I've found at least three issues: * extern C clauses are not recursively scanned * the first time we see a cursor with language != C, we stop with an exception (instead of skipping) * typing of enum constant is different in C++ and causes infinite recursion These issues were preventing jextract to work with real world libraries (I tested OpenGL) when `-xc++` was specified. Now jextract will try to parse as much as possible. Unrecognized constructs/languages will be skipped and a diagnostic will be reported by the log. ------------- Commit messages: - Add test - Initial push Changes: https://git.openjdk.org/jextract/pull/256/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=256&range=00 Issue: https://bugs.openjdk.org/browse/CODETOOLS-7903783 Stats: 198 lines in 8 files changed: 175 ins; 13 del; 10 mod Patch: https://git.openjdk.org/jextract/pull/256.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/256/head:pull/256 PR: https://git.openjdk.org/jextract/pull/256
On Thu, 1 Aug 2024 12:37:32 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
This PR fixes a bunch of issues with trying to run jextract in CPP mode, using a `compile_flags.txt` file containing the `-xc++` flag.
I've found at least three issues: * extern C clauses are not recursively scanned * the first time we see a cursor with language != C, we stop with an exception (instead of skipping) * typing of enum constant is different in C++ and causes infinite recursion
These issues were preventing jextract to work with real world libraries (I tested OpenGL) when `-xc++` was specified.
Now jextract will try to parse as much as possible. Unrecognized constructs/languages will be skipped and a diagnostic will be reported by the log.
src/main/java/org/openjdk/jextract/clang/CursorKind.java line 104:
102: */ 103: StaticAssert(CXCursor_StaticAssert()), 104: Unsupported(-1);
I've added this, otherwise jextract will crash every time it sees a cursor it doesn't understand, as `Parser` calls `getKind` on the cursors it sees. Alternatives would be to (a) add an enum constant here for all the cases libclang support, or (b) skip this enum entirely, and just use jextract constants. I've held off a bigger refactoring for now, but this feels an area where we seem to be creating more work for ourselves than needed. src/main/java/org/openjdk/jextract/impl/TreeMaker.java line 131:
129: private boolean isAllowedCXXDecl(Cursor cursor) { 130: return switch (cursor.kind()) { 131: case StaticAssert, StructDecl, UnionDecl -> true;
When in C++ mode, both structs and unions are reported to have language C++ (understandable, since they can also contain more members, such as methods). So we need to add a list of the constructs we like, regardless of the language. ------------- PR Review Comment: https://git.openjdk.org/jextract/pull/256#discussion_r1700070753 PR Review Comment: https://git.openjdk.org/jextract/pull/256#discussion_r1700072211
On Thu, 1 Aug 2024 12:40:35 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
This PR fixes a bunch of issues with trying to run jextract in CPP mode, using a `compile_flags.txt` file containing the `-xc++` flag.
I've found at least three issues: * extern C clauses are not recursively scanned * the first time we see a cursor with language != C, we stop with an exception (instead of skipping) * typing of enum constant is different in C++ and causes infinite recursion
These issues were preventing jextract to work with real world libraries (I tested OpenGL) when `-xc++` was specified.
Now jextract will try to parse as much as possible. Unrecognized constructs/languages will be skipped and a diagnostic will be reported by the log.
src/main/java/org/openjdk/jextract/impl/TreeMaker.java line 131:
129: private boolean isAllowedCXXDecl(Cursor cursor) { 130: return switch (cursor.kind()) { 131: case StaticAssert, StructDecl, UnionDecl -> true;
When in C++ mode, both structs and unions are reported to have language C++ (understandable, since they can also contain more members, such as methods). So we need to add a list of the constructs we like, regardless of the language.
Why is `StaticAssert` here as well? ------------- PR Review Comment: https://git.openjdk.org/jextract/pull/256#discussion_r1752284626
On Tue, 10 Sep 2024 16:25:32 GMT, Jorn Vernee <jvernee@openjdk.org> wrote:
src/main/java/org/openjdk/jextract/impl/TreeMaker.java line 131:
129: private boolean isAllowedCXXDecl(Cursor cursor) { 130: return switch (cursor.kind()) { 131: case StaticAssert, StructDecl, UnionDecl -> true;
When in C++ mode, both structs and unions are reported to have language C++ (understandable, since they can also contain more members, such as methods). So we need to add a list of the constructs we like, regardless of the language.
Why is `StaticAssert` here as well?
`StaticAssert` came from the code that was there before. The comment in the jextract code says: But libclang maps both C11's _Static_assert * and C++11's static_assert to same CursorKind So I just preserved what the code was already doing here. ------------- PR Review Comment: https://git.openjdk.org/jextract/pull/256#discussion_r1752433166
On Tue, 10 Sep 2024 17:57:56 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
Why is `StaticAssert` here as well?
`StaticAssert` came from the code that was there before. The comment in the jextract code says:
But libclang maps both C11's _Static_assert * and C++11's static_assert to same CursorKind
So I just preserved what the code was already doing here.
Ah, I see. Missed that! ------------- PR Review Comment: https://git.openjdk.org/jextract/pull/256#discussion_r1752659780
On Thu, 1 Aug 2024 12:37:32 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
This PR fixes a bunch of issues with trying to run jextract in CPP mode, using a `compile_flags.txt` file containing the `-xc++` flag.
I've found at least three issues: * extern C clauses are not recursively scanned * the first time we see a cursor with language != C, we stop with an exception (instead of skipping) * typing of enum constant is different in C++ and causes infinite recursion
These issues were preventing jextract to work with real world libraries (I tested OpenGL) when `-xc++` was specified.
Now jextract will try to parse as much as possible. Unrecognized constructs/languages will be skipped and a diagnostic will be reported by the log.
Marked as reviewed by jvernee (Committer). ------------- PR Review: https://git.openjdk.org/jextract/pull/256#pullrequestreview-2293022859
On Thu, 1 Aug 2024 12:37:32 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
This PR fixes a bunch of issues with trying to run jextract in CPP mode, using a `compile_flags.txt` file containing the `-xc++` flag.
I've found at least three issues: * extern C clauses are not recursively scanned * the first time we see a cursor with language != C, we stop with an exception (instead of skipping) * typing of enum constant is different in C++ and causes infinite recursion
These issues were preventing jextract to work with real world libraries (I tested OpenGL) when `-xc++` was specified.
Now jextract will try to parse as much as possible. Unrecognized constructs/languages will be skipped and a diagnostic will be reported by the log.
This pull request has been closed without being integrated. ------------- PR: https://git.openjdk.org/jextract/pull/256
participants (3)
-
duke
-
Jorn Vernee
-
Maurizio Cimadamore