New candidate JEP: 468: Derived Record Creation (Preview)

[Remi Forax]

> From: "Brian Goetz" <brian.goetz at oracle.com>
> To: "attila kelemen85" <attila.kelemen85 at gmail.com>
> Cc: "amber-dev" <amber-dev at openjdk.org>, "Gavin Bierman"
> <gavin.bierman at oracle.com>
> Sent: Tuesday, April 23, 2024 3:34:16 PM
> Subject: Re: New candidate JEP: 468: Derived Record Creation (Preview)
[promoted to amber-spec-expert because I think this discussion is quite interresting] 

> So, a further thing to keep in mind is that currently, adding fields to records
> is not even source compatible to begin with. For example, if we have

> record Point(int x, int y) { }

> And a client uses it in a pattern match:

> case Point(int x, int y):

> And then we add an `int z` component, the client will break. (When we are able
> to declare deconstruction patterns, such a migration could include an XY
> pattern as well as constructor, but we are not there yet.)

> I think your mail rests on the assumption that it should be fine to modify
> records willy-nilly and expect compatibility without a recompile-the-world, but
> I think this is a questionable assumption.

This is a reasonable assumption. Java is well know to be backward compatible, people will expect any new feature to be backward compatible too. 
As an example, the way enums were compiled in 1.5 was not bacward compatible if the enums were used in a switch. Later the translation strategy was changed to be backward compatible. 

One possible quick fix is to restrict the access. For sealed classes, we have restricted the permitted subclasses to be in the same package/same module to avoid such separate compilation issues. 

Do you think that introducing the same restriction on derived record creation is a good idea ? 

> Records will likely have features that ordinary classes do not yet have access
> to for a while, making such changes risky.

Yes, the idea of derived record creation is based on the fact that there is a canonical constructor + a way to deconstruct using accessors (for now). 
I do not see classes having canonical constructors in the future so yes, this feature is limited to records. 

Rémi 

>> On Apr 20, 2024, at 5:49 PM, Attila Kelemen < [
>> mailto:attila.kelemen85 at gmail.com | attila.kelemen85 at gmail.com ] > wrote:

>> I have a backward compatibility concern about this JEP. Consider that I have the
>> following record:

>> `record MyRecord(int x, int y) { }`

>> One day I realize that I need that 3rd property which I want to add in a
>> backward compatible way, which I will do the following way:

>> ```
>> record MyRecord(int x, int y, int z) {
>> public MyRecord(int x, int y) {
>> this(x, y, 0);
>> }
>> }
>> ```

>> As far as I understand, this will still remain binary compatible. However, if I
>> didn't miss anything, then this JEP makes it non-source compatible, because
>> someone might wrote the following code:

>> ```
>> var obj1 = new MyRecord(1, 2);
>> int z = 26;
>> var obj2 = obj1 with { y = z; }
>> ```

>> If this code is compiled again, then it will compile without error, but while in
>> the first version `obj2.y == 26`, now `obj2.y == 0`. This seems rather nasty to
>> me because I was once bitten by this in Gradle (I can't recall if it was Groovy
>> or Kotlin, but it doesn't really matter), where this is a threat, and you have
>> to be very careful adding a new property in plugin extensions with a too
>> generic name. Even though Gradle scripts are far less prone to this, since
>> those scripts are usually a lot less complicated than normal code.

>> I saw in the JEP that on the left hand side of the assignment this issue can't
>> happen, but as far as I can see the above problem is not prevented.

>> My proposal would be to, instead of shadowing variables, raise a compile time
>> error when the property name would shadow another variable. Though that still
>> leaves the above example backward incompatible, but at least I would be
>> notified of it by the compiler, instead of the compiler silently creating a
>> bug.

>> Another solution would be that the shadowing is done in the opposite order, and
>> the `int z = 26;` shadows the record property (with a potential warning). In
>> this case it would be even source compatible, if I didn't miss something.

>> Attila
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-spec-observers/attachments/20240423/2dff0b3f/attachment.htm>