Pattern matching on primitive types mxied with instanceof is a footgun
Victor Nazarov
asviraspossible at gmail.com
Wed Feb 5 06:21:28 UTC 2025
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-observers/attachments/20250205/c06e789f/attachment.htm>
More information about the amber-spec-observers
mailing list