Record copy()/with()
Tagir Valeev
amaembo at gmail.com
Sat May 23 15:30:45 UTC 2020
Hello, Sergei!
I see a number of problems with your proposed approach.
1. Naming. Java never specified any kind of derived identifiers using
prefixes/suffixes/etc. And, in my opinion, it's for good, because such
things make the spec dirty. You assume that every record component is
named in plain English but Java allows any language in identifiers.
The method name like with年齢 or withВозраст would not look right. Also,
it would be necessary to specify how uppercasing is performed. Not
every character valid in Java identifier has an uppercase equivalent.
Also, what if the record component name is already uppercase? What if
there are two record components that differ in the first letter case
only? Should we prohibit such records? What if the record component
name is a Turkish word that starts with lowercase 'i'? Should we apply
tr locale for changing the case yielding grammatically correct dotted
İ (but making the code generation locale-dependent) or should we use
root locale always producing grammatically incorrect dot-less I?
Unlike some other languages, Java is known for very strict spec, this
is the strong part of Java. Having all of these corner cases specified
makes me sad. Btw, such constructs add a pain for IDE developers as
well. Imagine that a user starts a rename refactoring at the wither
call.
2. Assuming that record components could be validated in the
constructor, not every record could be created using step-by-step
builders. Imagine if you have a record Point(int x, int y) with a
restriction that components must have an equal sign. You can create
from the scratch new Point(1, 1) or new Point(-1, -1), but new
Point(1, 1).withX(-1).withY(-1) would not work.
3. Record components could also be normalized in the constructor. E.g.
assume record Fraction(int numerator, int denominator) that normalizes
the components using GCD in the constructor. Using withers would
produce a weird result based on the previous value.
4. Points 2 and 3 may lead to the conclusion that not every record
actually needs copying. In fact, I believe, only a few of them would
need it. Adding them automatically would pollute the API and people
may accidentally use them. I believe, if any automatic copying
mechanism will be added, it should be explicitly enabled for specific
records.
With best regards,
Tagir Valeev.
On Sat, May 23, 2020 at 10:06 PM Sergei Egorov <bsideup at gmail.com> wrote:
>
> Hi Remi,
>
> Thanks for raising this important topic!
>
> I wonder if Records can provide the good old
> withName()/withAge()/withWhatever() methods and the compiler will either
> merge them or we let EA do the rest?
>
> On Sat, May 23, 2020 at 1:36 PM Remi Forax <forax at univ-mlv.fr> wrote:
>
> > I've spent a little time to see how we can provide a with()/copy() method,
> > that creates a new instance from an existing record changing the value of
> > several components in the process.
> >
> > It's a feature that was requested several times while i was presenting how
> > record works and Scala, Kotlin and C# all provide an equivalent.
> > And there is a way to add it without introducing a new syntax.
> >
> > Syntax in Scala or Kotlin:
> > val otherPerson = person.copy(name = "John", age = 17)
> >
> > Syntax in C#:
> > var otherPerson = person with { Name = "John", Age = 17 };
> >
> > One can note that the syntax in C# doesn't reuse the named argument syntax
> > of C#,
> > with the named arguments syntax, it should be
> > var otherPerson = person.copy(name: "John", age: 17);
> >
> >
> > For Java, one solution is to re-use the same trick used to by
> > MethodHandle.invoke*()/VarHandle.*, have a special syntax that is very
> > close to the actual syntax so the feature is nicely integrated with the
> > rest of the language. Here, the last time we discuss this, we stumble
> > because unlike with MethodHandle.invoke*(), the arguments are a key/value
> > pairs and there is no existing syntax for that currently in Java.
> >
> > I believe there possible trick here, use Object... as Map.of() does.
> > the idea is to add a method 'Record with(Object... componentValuePairs)'
> > in java.lang.Record, and ask the compiler to verify that the even arguments
> > (0, 2, 4, etc) are constant strings
> >
> > Proposed syntax:
> > var otherPerson = person.with("name", "John", "age", 17);
> >
> > Then the compiler translates the method call to an invokedynamic to a
> > bootstrap method
> > public static CallSite bsm(Lookup lookup, String name, MethodType type,
> > String[] componentNames)
> > the componentNames being "name" and "age" in the example. The methodType
> > has the record as first parameter type and return type, the other
> > parameters are the types of the record components corresponding to the
> > component names.
> >
> > In term of separate compilation, the BSM should verify that the component
> > names are valid record component and that the method type parameters has
> > the same type as the corresponding record component.
> > So adding a new record component is a compatible change, removing a record
> > component or changing its type is not if a method call to 'with' reference
> > it.
> >
> > In term of Class.getMethod()/Lookup.findVirtual(), I propose to not see
> > the method 'with', so it's just a compiler artifact, if someone want the
> > method 'with' at runtime, he can call the bootstrap method.
> >
> > regards,
> > Rémi
> >
> > PS: here is the code for the bootstrap method
> >
> > ---
> > public static CallSite bsm(Lookup lookup, String name, MethodType type,
> > String[] componentNames) {
> > Objects.requireNonNull(lookup);
> > Objects.requireNonNull(name);
> > if (type.parameterCount() == 0 || type.returnType() !=
> > type.parameterType(0)) {
> > throw new IllegalArgumentException("invalid method type " + type);
> > }
> > if (componentNames.length != type.parameterCount() - 1) { // implicit
> > null check
> > throw new IllegalArgumentException("wrong number of component names
> > ");
> > }
> >
> > HashMap<String, Integer> withIndexMap = new HashMap<>();
> > for (int i = 0; i < componentNames.length; i++) {
> > String componentName = Objects.requireNonNull(componentNames[i]);
> > Object result = withIndexMap.put(componentName, i + 1); // 'this' is
> > at position 0
> > if (result != null) {
> > throw new IllegalArgumentException(
> > "component names contains twice the same name " +
> > componentName);
> > }
> > }
> >
> > Class<?> recordType = type.returnType();
> > RecordComponent[] components = recordType.getRecordComponents();
> > if (components == null) {
> > throw new IllegalArgumentException("the return type is not a record
> > " + recordType.getName());
> > }
> >
> > int length = components.length;
> > Class<?>[] constructorParameterTypes = new Class<?>[length];
> > int[] reorder = new int[length];
> > MethodHandle[] filters = new MethodHandle[length];
> > for (int i = 0; i < length; i++) {
> > RecordComponent component = components[i];
> > String componentName = component.getName();
> > Class<?> componentType = component.getType();
> > constructorParameterTypes[i] = componentType;
> > int withIndex = withIndexMap.getOrDefault(componentName, -1);
> > // a record component value either comes from the arguments or from
> > this + accessor call
> > if (withIndex == -1) { // it comes from this + accessor
> > try {
> > filters[i] = lookup.unreflect(component.getAccessor());
> > } catch (IllegalAccessException e) {
> > throw (IllegalAccessError) new IllegalAccessError().initCause(e);
> > }
> > // and reorder[i] == 0
> > } else { // it comes from the arguments
> > reorder[i] = withIndex;
> > if (type.parameterType(withIndex) != componentType) {
> > throw new IncompatibleClassChangeError(
> > "invalid parameter type "
> > + componentType
> > + " at "
> > + withIndex
> > + " for component name "
> > + componentName);
> > }
> > withIndexMap.remove(componentName); // mark that the component
> > name has been visited
> > // and filter[i] == null
> > }
> > }
> >
> > if (!withIndexMap.isEmpty()) { // some component names do not exist
> > throw new IncompatibleClassChangeError("invalid component names " +
> > withIndexMap.keySet());
> > }
> >
> > MethodHandle constructor;
> > try {
> > constructor = lookup.findConstructor(recordType,
> > MethodType.methodType(void.class, constructorParameterTypes));
> > } catch (NoSuchMethodException e) {
> > throw (NoSuchMethodError) new NoSuchMethodError().initCause(e);
> > } catch (IllegalAccessException e) {
> > throw (IllegalAccessError) new IllegalAccessError().initCause(e);
> > }
> > MethodHandle filtered = MethodHandles.filterArguments(constructor, 0,
> > filters);
> > MethodHandle target = MethodHandles.permuteArguments(filtered, type,
> > reorder);
> > return new ConstantCallSite(target);
> > }
> >
> >
> >
> >
> >
> >
> >
> >
> >
More information about the amber-spec-observers
mailing list