Pattern matching on primitive types mxied with instanceof is a footgun

[Stephan Herrmann]

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
>>>
>