A small patch to use the StringConcatFactory instead of String.format() to implement toString() of a data class

[Vicente Romero]

Hi Remi,

Thanks for the patch, seems useful. We could add it as an optional 
implementation and make a comparison between the possible 
implementations. Another possible implementation, again in the case of 
the toString, is to generate an indy call directly from the data class 
to a StringConcatFactory method,

Thanks,
Vicente

On 11/12/2017 04:58 PM, Remi Forax wrote:
> Hi all,
> A small patch to use the StringConcatFactory to implement the toString of a data class.
>
> The patch manage the case where there is no getter like the current code, even if you can not create a data class with no field using javac.
>
> cheers,
> Rémi
>
> diff -r a6a70e4541b3 src/java.base/share/classes/java/lang/invoke/ObjectMethodBuilders.java
> --- a/src/java.base/share/classes/java/lang/invoke/ObjectMethodBuilders.java	Fri Nov 10 18:51:06 2017 +0100
> +++ b/src/java.base/share/classes/java/lang/invoke/ObjectMethodBuilders.java	Sun Nov 12 22:54:42 2017 +0100
> @@ -189,49 +189,50 @@
>   
>       /**
>        * Generates a method handle for the {@code toString} method for a given data class
> +     * @param lookup          the lookup object
>        * @param receiverClass   the data class
>        * @param getters         the list of getters
>        * @param names           the names
>        * @return the method handle
>        */
> -    private static MethodHandle makeToString(Class<?> receiverClass,
> +    private static MethodHandle makeToString(MethodHandles.Lookup lookup,
> +                                            Class<?> receiverClass,
>                                               List<MethodHandle> getters,
>                                               List<String> names) {
> -        // This is a pretty lousy algorithm; we spread the receiver over N places,
> -        // apply the N getters, apply N toString operations, and concat the result with String.format
> -        // Better to use String.format directly, or delegate to StringConcatFactory
> -        // Also probably want some quoting around String components
> +        // This is a simple algorithm; we spread the receiver over N places,
> +        // apply the N getters, then delegate to StringConcatFactory
> +        // We may want some quoting around String components ?
>   
>           assert getters.size() == names.size();
>   
> -        int[] invArgs = new int[getters.size()];
> -        Arrays.fill(invArgs, 0);
> -        MethodHandle[] filters = new MethodHandle[getters.size()];
> -        StringBuilder sb = new StringBuilder();
> -        sb.append(receiverClass.getSimpleName()).append("[");
> -        for (int i=0; i<getters.size(); i++) {
> -            MethodHandle getter = getters.get(i); // (R)T
> -            MethodHandle stringify = stringifier(getter.type().returnType()); // (T)String
> -            MethodHandle stringifyThisField = MethodHandles.filterArguments(stringify, 0, getter);    // (R)String
> -            filters[i] = stringifyThisField;
> -            sb.append(names.get(i)).append("=%s");
> -            if (i != getters.size() - 1)
> -                sb.append(", ");
> -        }
> -        sb.append(']');
> -        String formatString = sb.toString();
> -        MethodHandle formatter = MethodHandles.insertArguments(STRING_FORMAT, 0, formatString)
> -                                              .asCollector(String[].class, getters.size()); // (R*)String
> -        if (getters.size() == 0) {
> -            // Add back extra R
> -            formatter = MethodHandles.dropArguments(formatter, 0, receiverClass);
> -        }
> -        else {
> -            MethodHandle filtered = MethodHandles.filterArguments(formatter, 0, filters);
> -            formatter = MethodHandles.permuteArguments(filtered, MethodType.methodType(String.class, receiverClass), invArgs);
> +        if (getters.isEmpty()) {
> +            return MethodHandles.dropArguments(
> +                MethodHandles.constant(String.class, "[]"),
> +                0, receiverClass);
>           }
>   
> -        return formatter;
> +        Class<?>[] getterTypes = new Class<?>[getters.size()];
> +        StringBuilder recipeBuilder = new StringBuilder();
> +        recipeBuilder.append(receiverClass.getSimpleName()).append('[');
> +        String separator = "";
> +        for (int i = 0; i < getters.size(); i++) {
> +            recipeBuilder.append(separator).append(names.get(i)).append("=\1");
> +            getterTypes[i] = getters.get(i).type().returnType();
> +            separator = ", ";
> +        }
> +        String recipe = recipeBuilder.append(']').toString();
> +        MethodType concatType = MethodType.methodType(String.class, getterTypes);
> +
> +        CallSite cs;
> +        try {
> +            cs = StringConcatFactory.makeConcatWithConstants(lookup, "toString", concatType, recipe);
> +        } catch (StringConcatException e) {
> +            throw (LinkageError)new LinkageError().initCause(e);
> +        }
> +        MethodHandle target = cs.dynamicInvoker();
> +        target = MethodHandles.filterArguments(target, 0, getters.toArray(new MethodHandle[0]));
> +        target = MethodHandles.permuteArguments(target, MethodType.methodType(String.class, receiverClass), new int[getters.size()]);
> +        return target;
>       }
>   
>       /**
> @@ -277,6 +278,6 @@
>        */
>       public static CallSite makeToString(MethodHandles.Lookup lookup, String invName, MethodType invType,
>                                           Class<?> dataClass, String names, MethodHandle... getters) throws Throwable {
> -        return new ConstantCallSite(makeToString(dataClass, List.of(getters), List.of(names.split(";"))));
> +        return new ConstantCallSite(makeToString(lookup, dataClass, List.of(getters), List.of(names.split(";"))));
>       }
>   }