Bug in the record pattern translation / record pattern spec
Brian Goetz
brian.goetz at oracle.com
Mon Jun 27 19:38:00 UTC 2022
[ dropping amber-dev ]
Thanks for this concrete example, its helpful.
So far, we can put this in the category of "there exists at least one
case where this handling of exceptions makes it harder to understand
what is going on." This isn't necessarily dispositive, in that in order
to make a fair determination, we'd have to consider both the frequency
(this is kind of a corner case) and also the complement: the cases where
it helps.
Let me explain (again) why we went this way. We're doing record
patterns *now*, but it is entirely clear where this is going: record
patterns are not special, records will eventually acquire an implicit
canonical deconstruction pattern, and users will be able to explicitly
declare deconstruction patterns as well. And (barring separate
compilation anomalies), the two should not really be distinguishable;
whether `javac` generates a synthetic member for a dtor pattern, or
whether it simulates the behavior of that synthetic member on the client
side, should be an implementation detail. So accessors are not
"special" here.
Having a deconstruction pattern throw is a logic error, of the same sort
that causes NPE or AIOOBE. Pattern matching may, as part of doing its
thing, invoke the imperative logic associated with a deconstruction
pattern; it expects these not to fail. Telling users "pattern matching
might fail with an arbitrary error" only encourages users to try and
catch these errors, which is misguided and which might well catch up
other unrelated errors in the same net. "There was a violated invariant
in a pattern-driven construct" is a reasonable error model to present to
users when we are invoking unspecific behavior at unspecific times on
the user's behalf.
Obviously, the code in this example is broken; what you're hoping for is
a clean SOE (if it were wrapped once with ME, that would be fine), so
users can say "oops, I'm a dummy" and fix it.
On 6/27/2022 4:54 AM, Remi Forax wrote:
> Hi all,
> i've already reported that issue but it was theoretical because the support of the record pattern was not integrated into the jdk 19 at that time and i was not able to explain int clearly. Now that the support is available, this code should throw a StackOverflow error but it creates a linked list of MatchException which makes it hard to debug.
>
> Wrapping all exceptions into a runtime exception is not a good idea when you have recursive calls which is a kind of natural when you have pattern matching, for me it's a spec issue but i want to be sure.
>
> BTW, the bug here lies in the fact that deconstructing Cons(int value, int size, RecChain next) calls the accessor size() which itself calls RecChain::size which is a nice puzzler by itself.
>
> public sealed interface RecChain {
> default int size() {
> return switch (this) {
> case Nil __ -> 0;
> case Cons(int value, int size, RecChain next) -> 1 + next.size();
> };
> }
>
> record Nil() implements RecChain { }
>
> record Cons(int value, int size, RecChain next) implements RecChain {
> @Override
> public int size() {
> return size != -1? size: RecChain.super.size();
> }
> }
>
> public static void main(String[] args){
> RecChain chain = new Nil();
> for (var i = 0; i < 100; i++) {
> chain = new Cons(i, -1, chain);
> }
> System.out.println(chain.size());
> }
> }
>
> regards,
> Rémi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-spec-experts/attachments/20220627/67f8b1ba/attachment.htm>
More information about the amber-spec-experts
mailing list