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

[Attila Kelemen]

>
> I can see how this would be an attractive idea, but its not practical.  I
> think this idea rests on some assumptions that are not true, that you can
> “just” rename a conflicting local out of the way, and secondarily that in
> the event that a record component is shadowed, you can “just” access it via
> the record.  The first is simply not true; we’ll come back for the second
> in a bit.
>
> Suppose I have some records:
>
>      record A(int x, B b) { }
>      record B(int x) { }
>
>      A a = …
>      a = a with { b with { x = 3; } }
>
> In the outer reconstruction block, we have component variables x and b; in
> the inner block, we have a component variable x that shadows the outer x.
> Neither of these can be renamed “out of the way”.  Under your proposal, we
> wouldn’t be able to use nested reconstruction on A at all, which is pretty
> bad.
>

I admit, I should have been clearer here, because there is an asymmetry
here. I'm not talking about the left hand side (your example above should
compile fine). In fact, I believe the left hand side is addressed perfectly
by the JEP. That is, on the left hand side it does not allow anything
outside the curlies of the `with` (so there can be no problem with that).
That is, the compile time error would not make nesting impossible.

So, the question is how the expression "x" is treated. In fact, such a
nested case is the most error prone and requires being explicit about your
choice of which "x" expression are we talking about. For instance, let's
look at a modified example (to contain such conflicts):

```
record A(int x, int y, A other) { }
A a = ...; // assume a.other != null
a = a with { other = other.with { x = y; } }
```

I think in the above nested example, it is less confusing if
we disambiguate by writing `other.y`. The reason being is that if someone
reads the code then there is always a decent chance to notice a variable in
an outer scope and then erroneously just assume the variable is what you
have noticed. Especially, when different types and many properties are
involved (or simply due to asymmetric familiarity with the types involved).

A similar example is:
>
>     record Person(String name, Person parent) { }
>
> I think these examples shows that “just make it an error” is a
> non-starter.
>
> As to “require re-access through the record”, this is a bad road to go
> down.  It is highly error-prone, since you now have two ways to access the
> same logical thing, but they’re not the same actual thing — one’s a copy,
> and it might have been mutated since copying.  So accessing the original in
> this context would be questionable.  And further, when we extent
> reconstruction to classes as well as records, the “I can just get it from
> the record” claim becomes no longer true.
>

 Just to make it explicit, I'm assuming you are referring to a situation
like this (and also assuming there is a name conflict with the outer scope):

```
a with {
  x = y;
  y = x; // not that same as `a.x`
}
```

Though this can be a source of error for sure, but I think this can be a
source of error both ways with roughly equal likelihood (so balances out),
because in the above example, maybe you wanted to exchange `x` and `y` and
this was a mistake. In fact, my opinion is that when you are not referring
to the original value, then it would be a better practice to have the value
in an intermediate variable anyway. Because most certainly nobody would
question the intent of this:

```
a with {
  int commonValue = y;
  x = commonValue ;
  y = commonValue;
}
```

That is, I would certainly waste more of my time to check if the intent of
the `x = y; y = x;` block is what it does. Of course, as I hinted
previously there can be examples where there is probably no such confusion,
like with:

```
a with {
  x = 5;
  y = x;
}
```

However, my point here is that I don't see that using `original.myProperty`
is in general more error prone. Especially since this is only a question if
there is a naming conflict.

Also, there is one extra argument that I forgot to write down in my
previous email. I would only expect the problem I originally wrote to
happen for projects that are large evolving production systems (since it
requires independent dependency upgrades). Now, those systems are probably
not keen to use preview features (since it would be horrible for them to
back out from the said preview feature). So, if my stated problem is indeed
a problem that will not be experienced by people trying out this in
preview. However, it is more likely that people will run into the shadowing
issues (since that requires less unusual situations). And if the
compilation error turns out to be a burden, then you can't just not notice
the pain. However, in the other case, it is very unusual for someone to
remember that "if this was a compile time error, I would have been very
sad" (the actual pain is just a lot more memorable). So, I think making it
a compile time error for preview brings the additional benefit of seeing if
the compile time error actually hurts.

Anyway, I think I wrote down all of my arguments now. So, I don't think I
will have any more notes on the topic. I personally think that it is
overall not terrible either way (definitely wouldn't be the worst unfixable
mistake of Java, if shadowing indeed turned out to be a mistake), but I do
think it would be better with the compile time error.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20240424/33e3194e/attachment-0001.htm>