RFR: 7903637: Use holder classes for string constants
Maurizio Cimadamore
mcimadamore at openjdk.org
Mon Jan 22 18:45:55 UTC 2024
On Mon, 22 Jan 2024 18:35:24 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:
> Use holder class idiom for string constants (See JBS issue for motivation).
>
> The code in `emitConstant` is a bit copy-pasty. We need to strike a balance between giving a clear picture of the generated code by using a single string template as much as possible, and reducing duplication by splitting the templates up and weaving in more control flow.
>
> I went with the repeated, but clearer approach here of having 2 separate templates. Let me know if you'd like me to change that.
src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 397:
> 395: private void emitConstant(Class<?> javaType, String constantName, Object value, Declaration declaration) {
> 396: incrAlign();
> 397: boolean useHolderClass = value instanceof String;
maybe `isString` would be a better name
src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 424:
> 422: private String constantValue(Class<?> type, Object value) {
> 423: if (value instanceof String) {
> 424: return STR."\{runtimeHelperName()}.LIBRARY_ARENA.allocateFrom(\"\{Utils.quote(Objects.toString(value))}\")";
I think I'd prefer to inline this in the above method `emitConstant` - as we're not really supposed to call allocation methods from plain field initializers (e.g. this is only for holder class). E.g. I'd prefer jextract to thrown an exception if we ever get here from outside an holder class.
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/191#discussion_r1462263313
PR Review Comment: https://git.openjdk.org/jextract/pull/191#discussion_r1462262995
More information about the jextract-dev
mailing list