RFR: Make method handle accessor private
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`. This was discovered when manually eyeballing at jextracted code for windows.h. ------------- Commit messages: - Initial push Changes: https://git.openjdk.org/jextract/pull/188/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=188&range=00 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jextract/pull/188.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/188/head:pull/188 PR: https://git.openjdk.org/jextract/pull/188
On Fri, 19 Jan 2024 16:51:02 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`.
This was discovered when manually eyeballing at jextracted code for windows.h.
Marked as reviewed by jvernee (Committer). My comment got dropped it seems. What do you think about `MEMBER_MODS`? This is not something that is variable WRT the template text. I think I'd personally rather see the expanded code in the template, so that it's easier to see what is being generated. Though, theoretically, this makes it harder to bulk update all the access modifiers. But, that seems pretty rare any way? P.S. and, in this case I don't think the variable name `MEMBER_MODS` adds any clarity to the code. ------------- PR Review: https://git.openjdk.org/jextract/pull/188#pullrequestreview-1833739260 PR Comment: https://git.openjdk.org/jextract/pull/188#issuecomment-1901124954
On Fri, 19 Jan 2024 21:15:35 GMT, Jorn Vernee <jvernee@openjdk.org> wrote:
My comment got dropped it seems.
What do you think about `MEMBER_MODS`? This is not something that is variable WRT the template text. I think I'd personally rather see the expanded code in the template, so that it's easier to see what is being generated.
Though, theoretically, this makes it harder to bulk update all the access modifiers. But, that seems pretty rare any way?
P.S. and, in this case I don't think the variable name `MEMBER_MODS` adds any clarity to the code.
I agree, will replace. ------------- PR Comment: https://git.openjdk.org/jextract/pull/188#issuecomment-1901179270
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`.
This was discovered when manually eyeballing at jextracted code for windows.h.
Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comments ------------- Changes: - all: https://git.openjdk.org/jextract/pull/188/files - new: https://git.openjdk.org/jextract/pull/188/files/4c77ae51..a7f001ca Webrevs: - full: https://webrevs.openjdk.org/?repo=jextract&pr=188&range=01 - incr: https://webrevs.openjdk.org/?repo=jextract&pr=188&range=00-01 Stats: 18 lines in 1 file changed: 0 ins; 12 del; 6 mod Patch: https://git.openjdk.org/jextract/pull/188.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/188/head:pull/188 PR: https://git.openjdk.org/jextract/pull/188
On Fri, 19 Jan 2024 22:14:13 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`.
This was discovered when manually eyeballing at jextracted code for windows.h.
Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
Address review comments
Marked as reviewed by jvernee (Committer). src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 164:
162: emitDocComment(decl); 163: appendLines(STR.""" 164: \{"public static"} \{retType} \{javaName}(\{paramExprs(declType, finalParamNames, isVarArg)}) {
I suppose this should just be: Suggestion: public static {retType} {javaName}({paramExprs(declType, finalParamNames, isVarArg)}) { ------------- PR Review: https://git.openjdk.org/jextract/pull/188#pullrequestreview-1833875502 PR Review Comment: https://git.openjdk.org/jextract/pull/188#discussion_r1459915961
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`.
This was discovered when manually eyeballing at jextracted code for windows.h.
Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix string template ------------- Changes: - all: https://git.openjdk.org/jextract/pull/188/files - new: https://git.openjdk.org/jextract/pull/188/files/a7f001ca..a27e0b50 Webrevs: - full: https://webrevs.openjdk.org/?repo=jextract&pr=188&range=02 - incr: https://webrevs.openjdk.org/?repo=jextract&pr=188&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jextract/pull/188.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/188/head:pull/188 PR: https://git.openjdk.org/jextract/pull/188
On Fri, 19 Jan 2024 16:51:02 GMT, Maurizio Cimadamore <mcimadamore@openjdk.org> wrote:
In my previous PR, I forgot to also make the method handle accessor (for native downcalls) `private`.
This was discovered when manually eyeballing at jextracted code for windows.h.
This pull request has now been integrated. Changeset: 14f12739 Author: Maurizio Cimadamore <mcimadamore@openjdk.org> URL: https://git.openjdk.org/jextract/commit/14f12739f59750340233870574c19d2dffd5... Stats: 14 lines in 1 file changed: 0 ins; 12 del; 2 mod Make method handle accessor private Reviewed-by: jvernee ------------- PR: https://git.openjdk.org/jextract/pull/188
participants (2)
-
Jorn Vernee
-
Maurizio Cimadamore