Pattern matching on primitive types mxied with instanceof is a footgun
Stephan Herrmann
stephan.herrmann at berlin.de
Tue Feb 4 21:57:32 UTC 2025
As part of educating people to prefer var in such cases, compilers/linters could
issue a warning when a nested primitive pattern is conditional. IDEs would then
offer to resolve the issue by changing to var or to the exact type.
Stephan
Am 04.02.25 um 18:08 schrieb Gavin Bierman:
> One good technique is to always favour `var` patterns; then you don’t get any
> unwanted tests, i.e.
>
> for(Spaceship spaceship : ...) {
> ...
> if (spaceship instanceof AlienSpaceship(var x, var y) {
> // display the alien space ship at (x, y)
> }
> ...
> }
>
> Gavin
>
>> On 4 Feb 2025, at 16:55, Maurizio Cimadamore <maurizio.cimadamore at oracle.com>
>> wrote:
>>
>> Hi,
>> would some of the comments in this thread change if the example was more like:
>>
>> AlienSpaceShip ship = ...
>> int x = (int)ship.getX();
>>
>> This works and no error is issued.
>>
>> From a design perspective, the above code is basically what the record pattern
>> is supposed to desugar to - in other words, pattern matching is precondition
>> for safe cast. For primitives, determining whether a cast is "safe" involves a
>> runtime range check, which is invoked by the compiler.
>>
>> So
>>
>> ```
>> jshell> double d = 4.2;
>> d ==> 4.2
>>
>> jshell> d instanceof int
>> $2 ==> false
>>
>> jshell> double d = 4.0;
>> d ==> 4.0
>>
>> jshell> d instanceof int
>> $4 ==> true
>> ```
>>
>> 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.)
>>
>> Maurizio
>>
>> 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
>>>
>
More information about the amber-spec-experts
mailing list