Pattern matching on primitive types mxied with instanceof is a footgun

Wed Feb 5 07:16:15 UTC 2025

Yes, Maurizio raised that at the end of his message. I think this
particular example is constructed to preclude that remedy, as it happens to
not be an unconditional pattern *either way*.

On Tue, Feb 4, 2025 at 10:21 PM Victor Nazarov <asviraspossible at gmail.com>
wrote:

> Isn't the problem here is that if-statement was used?
>
> From my understanding the intention of the code was clear: deconstruct
> this value without losing any information, but the problem is that we lack
> anything in the language to express this.
>
> What was supposed to be written was:
>
> let AlienSpaceship(int x, int y) = spaceship;
>
> The problem is that since we lack let-statement people started to see
> if-statement as a direct substitution, but it never was, at least not fully.
> --
> Victor Nazarov
>
> On Tue, 4 Feb 2025, 23:44 , <forax at univ-mlv.fr> wrote:
>
>>
>>
>> ------------------------------
>>
>> *From: *"Kevin Bourrillion" <kevinb9n at gmail.com>
>> *To: *"Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>> *Cc: *"Tagir Valeev" <amaembo at gmail.com>, "Brian Goetz" <
>> brian.goetz at oracle.com>, "Remi Forax" <forax at univ-mlv.fr>,
>> "amber-spec-experts" <amber-spec-experts at openjdk.java.net>
>> *Sent: *Tuesday, February 4, 2025 6:14:53 PM
>> *Subject: *Re: Pattern matching on primitive types mxied with instanceof
>> is a footgun
>>
>> On Tue, Feb 4, 2025 at 8:55 AM Maurizio Cimadamore <
>> maurizio.cimadamore at oracle.com> wrote:
>>
>>> This seems consistent. So the question is: what is the "mistake" we're
>>> talking about here? The user thinking it was a _unconditional_ match where
>>> in reality that's not the case? While in the case of instanceof
>>> unconditionality is harder to spot, if this was a switch, the compiler
>>> would have immediately asked the user to provide a default?
>>>
>>> It seems to me that the reasoning here implies that developers will use
>>> "if" + "instanceof" _without an "else" just for the purpose of extract some
>>> data, and then complain when the thing doesn't match (because they have no
>>> "else", and so their code will fail). It seems like our best weapon in this
>>> case is education, rather that introducing arbitrary restrictions and
>>> asymmetries in the language?
>>>
>>> (Or, perhaps, in the future have a way to perform an _unconditional_
>>> extraction without an "if", in which case the compiler will bark if the
>>> pattern is not unconditional.)
>>>
>> In some cases this will be relevant, but in *this *particular example
>> we're looping over Spaceship and then checking for AlienSpaceship, so, no
>> help.
>>
>> I think it is fair to acknowledge that this is a weakness of the feature
>> design. How to mitigate it? We can try to evangelize always preferring
>> `var` for nested type patterns that are expected to be unconditional.
>> That's an unfortunate readability trade-off, though. And a static analyzer
>> can't know whether you expected it to be unconditional or not, so there
>> might not be much it can do to help.
>>
>> I am not even sure what language change could be considered here. Note
>> that this problem is not specific to primitive types, it's just that
>> primitive types are currently the only ones that support "value conversion"
>> so we're encountering this issue with them first. Post-Valhalla, other
>> types might support value conversion too.
>>
>>
>> For me it's more about the conversion being implicit vs explicit.
>> Here, we have an implicit conversion while the conversion does not always
>> succeed.
>>
>> I believe such conversions, the one that may not always succeed, should
>> be explicit not implicit.
>>
>> regards,
>> Rémi
>>
>> @Brian, i'm more worry about a TA with more than 10 years of experience
>> having trouble to find a bug in a student project written in Java.
>> @Tagir, i'm not sure usin floating point values is the issue here, you
>> can have the same kind of bug between a character and an int.
>>
>>
>>
>>
>> On 04/02/2025 16:00, Tagir Valeev wrote:
>>>
>>> Hello!
>>>
>>> Well, I would say that Remi has a point. This is a silent mistake that
>>> can be made. You don't see the declaration, as it's in another file so you
>>> may forget about original types and assume that they are ints. The code
>>> compiles and doesn't even throw an exception, it just works incorrectly. It
>>> would be justified if such code pattern (converting from double to int only
>>> when double fits the int) was common, so conditional narrowing could be
>>> useful in many other places. But the fact is that it's highly uncommon.
>>> Nobody does this. Many other pattern matching features are nice. E.g., I
>>> often see the code which can be migrated to pattern switch (we are still on
>>> Java 17) and I really want to use it. However, I never see the code which
>>> can be migrated to a narrowing primitive pattern.
>>>
>>> I don't agree that we should drop primitive patterns completely, but we
>>> may reconsider floating point <-> integral conversions and disable them in
>>> patterns for a while (enabling them back later will be always possible).
>>>
>>> With best regards,
>>> Tagir Valeev
>>>
>>> On Tue, Feb 4, 2025 at 12:45 PM Brian Goetz <brian.goetz at oracle.com>
>>> wrote:
>>>
>>>> Wow, that was pretty amazing. You went from “one student, who was never
>>>> taught about how this works, was confused by it” to “ let’s pull the whole
>>>> feature “ in one breath.
>>>>
>>>> > On Feb 4, 2025, at 11:33 AM, Remi Forax <forax at univ-mlv.fr> wrote:
>>>> >
>>>> > Last week,
>>>> > one of the TA for the course "OOP using Java" was grading student
>>>> projects,
>>>> > the subject of the project can be summarize to "you are alone in
>>>> space and angry aliens want you to die".
>>>> >
>>>> > One of the project had a weird bug when displaying aliens.
>>>> > Spoiling the result of bug hunting, this is due to students using the
>>>> pattern matching on primitive without them realizing it.
>>>> >
>>>> > Here is a reproducer of the bug:
>>>> >
>>>> > // in AlienSpaceship.java
>>>> > record AlienSpaceship(double x, double y) implements Spaceship {
>>>> >  ...
>>>> > }
>>>> >
>>>> > ...
>>>> >
>>>> > // in a method in another class
>>>> >  ...
>>>> >  for(Spaceship spacehip : ...) {
>>>> >    ...
>>>> >    if (spaceship instanceof AlienSpaceship(int x, int y) {
>>>> >      // display the alien space ship at (x, y)
>>>> >    }
>>>> >    ...
>>>> >  }
>>>> >
>>>> > The TA said she was about to give up when she found the issue and
>>>> that the Java spec should not allow such pattern.
>>>> > I agree with here, pattern matching on primitive types is not a
>>>> feature people ask for and mixed with instanceof it's a footgun.
>>>> >
>>>> > This feature can be re-introduced later using method patterns and it
>>>> will be easier to understand the code,
>>>> > by example with an hyphotethical method Double.asInt
>>>> >
>>>> >  if (spaceship instanceof AlienSpaceship(Double.asInt(int x),
>>>> Double.asInt(int y)) {
>>>> >    ...
>>>> >
>>>> > regards,
>>>> > Rémi
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-spec-experts/attachments/20250204/9fcf6dd6/attachment.htm>