Add an option for copying comments in a header file over to the generated bindings - #308
Add an option for copying comments in a header file over to the generated bindings#308xpple wants to merge 5 commits into
Conversation
|
👋 Welcome back xpple! A progress list of the required criteria for merging this PR into |
|
@xpple This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. ➡️ To flag this PR as ready for integration with the above commit message, type |
Webrevs
|
|
There is currently a bug where copied comments could contain For what it's worth, I think using Markdown comments is a great idea. It would break compatibility with JDK 22, but I don't think retaining that compatibility is a goal. Footnotes
|
|
Mailing list message from Jonathan Strauss on jextract-dev: I did similar work on this in a private fork, I ran into a few traps, I can't recall what it was but just keep that in mind. If I have time I can dig some of it up. On Sun, Aug 2, 2026, at 5:05 PM, Frederik van der Els wrote: |
|
Please do! I believe everything to be working currently, but it's possible I missed something. |
This PR adds an option for copying comments in a header file over to the generated bindings. See also my previous email.
The option can be enabled by passing the
--copy-commentsflag to jextract. This flag is optional, and false by default. This means that if the flag is not passed, the output from jextract will be identical to what it was before this change.I decided against using the existing clang API for Doxygen comments, because they are far from standardized in C projects. This meant though that I had to come up with custom logic for associating comments with declarations. I am happy to incorporate feedback on how this association can be improved. Currently, all comments that immediately precede a declaration are associated with that declaration. Whitespace is ignored.
For example, in the below code, both comments are associated with the
funcdeclaration.For structs/unions, comments that precede member declarations will be associated with that member.
Similarly, enum constants can have comments too. Comments at the enum tag are ignored, because the enum tag itself is not retained in the jextract bindings.
In the API, the
Declarationinterface now has aList<String> comments()method. These are the raw comments, so they still contain the comment delimiters (//or/* ... */). If--copy-commentsis not passed, this method will return an empty list (notnull).In the binding generation code, I made an attempt to detect some common styles for comments, and account for them separately. For example, if a comment starts with three forward slashes, all three will be stripped (and not just the two that lexically define the comment). The code for this is in
ClassSourceBuilder#copyComments. Below is what the JavaDoc will look like for the above three examples.func
Color
Point
Not all JavaDoc comments pertaining to a certain declaration get the comments associated with that declaration. For example, for functions the
$descriptor,$handleand$addressmethods do not get the comments, and for structs the$layoutand$offsetmethods do not get them. The documentation is usually only relevant for calling/accessing/modifying something, not for derived properties.The comment association code works by keeping track of the previous cursor's extent's ending, and tokenizing between that and the current cursor's extent's beginning. For example, if in the below code the current cursor is
func2, then the tokens encountered will be;,// commentandvoid.These tokens will be iterated over in reverse order, ignoring the first token(s) (here only
void) as they are part of the current declaration. Then, as long as the encountered token is a comment (CXToken_Comment), it will be associated with the declaration. In cases where this approach can't work (e.g. there is no previous declaration, the previous declaration is in a different file or the previous declaration actually appears after the current declaration lexically), the code will resort to a fallback.Since I needed
clang_getRange, which wasn't in the symbols list (updateclang/clang.symbols) yet, I had to regenerate the bindings. Because jextract had received some updates since the last time the bindings were generated, there are some bindings changes unrelated toclang_getRangeas well. While updating, I noticed the instructions had gotten slightly out of date, so I updated those as well. I added three patch files which people cangit applyto apply the necessary changes more easily.I made sure to maintain source and binary compatibility in the public API of jextract.
I did not test the PR locally, but the tests I added passed on GitHub Actions. As this is my first time working with libclang, I am happy receive feedback for this PR.
Progress
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jextract.git pull/308/head:pull/308$ git checkout pull/308Update a local copy of the PR:
$ git checkout pull/308$ git pull https://git.openjdk.org/jextract.git pull/308/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 308View PR using the GUI difftool:
$ git pr show -t 308Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jextract/pull/308.diff
Using Webrev
Link to Webrev Comment