RFR: JDK-8285932 Implementation of JEP-430 String Templates (Preview) [v3]
Jim Laskey
jlaskey at openjdk.org
Fri Oct 28 19:41:33 UTC 2022
On Fri, 28 Oct 2022 18:45:05 GMT, Rémi Forax <forax at openjdk.org> wrote:
>> Jim Laskey has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Update TemplateRuntime::combine
>
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 32:
>
>> 30:
>> 31: import java.io.IOException;
>> 32: import java.util.*;
>
> Please do not use import *.
Changing.
> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 36:
>
>> 34: import java.lang.invoke.MethodHandles.Lookup;
>> 35: import java.lang.template.StringTemplate;
>> 36: import java.util.*;
>
> Another import * here
Changing
> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 118:
>
>> 116: * @since 20
>> 117: */
>> 118: public static final int MAX_INDY_CONCAT_ARG_SLOTS = 200;
>
> I do not think it's a good idea to make that constant available for everybody given that it's an artefact of the implementation.
There have been several requests to make it public in the past. You really can't use the methods in this class unless you know the value. Better to have the value exposed instead of developers transcribing the value into their code.
> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 999:
>
>> 997: * Promote integral types to int.
>> 998: */
>> 999: private static Class<?> promoteIntType(Class<?> t) {
>
> promoteToIntType ?
Changing
> src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 1060:
>
>> 1058: throws StringConcatException
>> 1059: {
>> 1060: Objects.requireNonNull(fragments, "fragments is null");
>
> I think you need to do some defensive copy here
>
> ptypes = List.copyOf(pTypes);
>
> to avoid the types and fragments to be changed at the same time they are checked.
Changing
-------------
PR: https://git.openjdk.org/jdk/pull/10889
More information about the compiler-dev
mailing list