[patterns] Unchecked cast warning is absent?

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
> À: forax at univ-mlv.fr
> Cc: "Tagir Valeev" <amaembo at gmail.com>, "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Mardi 3 Octobre 2017 12:17:31
> Objet: Re: [patterns] Unchecked cast warning is absent?

> On 03/10/17 10:47, forax at univ-mlv.fr wrote:
>>
>> ----- Mail original -----
>>> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>>> À: "Remi Forax" <forax at univ-mlv.fr>
>>> Cc: "Tagir Valeev" <amaembo at gmail.com>, "amber-dev" <amber-dev at openjdk.java.net>
>>> Envoyé: Mardi 3 Octobre 2017 10:57:07
>>> Objet: Re: [patterns] Unchecked cast warning is absent?
>>> On 03/10/17 09:16, Remi Forax wrote:
>>>> Maurizio,
>>>> __matches <=> instanceof + cast,
>>>> the instanceof part is currently illegal, so the implication is that __matches
>>>> should be illegal too.
>>>  From a code gen perspective I agree with you. From a type checking
>>> perspective I don't otherwise you would make pattern matching
>>> incompatible with generics:
>>>
>>> List<String> ls = ...
>>> if (x matches ArrayList<String> a) {
>>>    ...
>>> } else if (x matches LinkedList<String> ll) {
>>>    ...
>>> }
>>>
>>> I think accepting programs such as these is useful (and actually the
>>> above is not even an unsafe program!). But, using your strict
>>> interpretation, I can't do an instanceof whose target type is
>>> ArrayList<String> - so this should be rejected?
>> written with instanceof + cast, it's
>>
>> if (x instanceof ArrayList) {
>>    ArrayList<String> a = (ArrayList<String>)ls;
>>    ...
>> } else {
>>    if (x instanceof LinkedList) {
>>      LinkedList<String> a = (LinkedList<String>)ls;
>>      ...
>>    }
>> }
>>
>> which is as you said is ok.
> Sorry - I think that's not a valid argument :-)
> 
> You claimed that 'x matches T' should be rejected on the basis that
> 'instanceof T' is not valid.
> 
> But now you seem to imply that 'x matches C<X>' should be valid on the
> basis that 'instance of erasure(C<X>)' is valid.
> 
> If you use erasure for the instanceof step, you can do that with both
> type-variables and generic types. If you do not use erasure then you end
> up with illegal code for both.

i think we can acknowledge that type variables and parameterized types are different beasts even if they both have erasure,
you may want to do a typecheck on the erased part of a parameterized type which is typesafe (as in the example you provide), you have no such thing with a type variable.

so it may make sense to have a different behaviors for type variables and parameterized types.  

That's said, i prefer my proposal below.

>>
>> A quick idea is to try to separate how we handle type variables from how we
>> handle parameterized types.
>> But i think there is a better idea.
>>
>> One interesting things with __matches is that this is not the only mechanism to
>> match a type, one can use instanceof + cast,
>> so you can only allow __match and pattern matching if it's typesafe (no warning)
>> and let people write instanceof + cast if it's unsafe.
>>
>> It's like the enhanced for, you can not write a loop that need an index but that
>> fine because you can write the classical for loop in that case.
>>
>> So in my opinion, __match and the pattern matching should only work if it's
>> typesafe, so people reading the code will just think it's just a dumb type
>> cases,
>> if it's not safe, lets people think twice about that and write the code with
>> instanceof and cast to see on which branches they want to hang themselves.
> Sure, that's a possible way to spin it - but it still requires to apply
> a cast conversion to determine whether such conversion is safe or not
> (and if not, instead of a warning, you get an error).

yes, the check can also be refactored to ask if something an instanceof is safe or not and print a warning or an error depending on the syntax that using it.
But that's an implementation detail, in term of semantics, the idea is that __match and the pattern matching only allow typesafe syntax.

Actually, most of the places where you really need unsafe casts is when you deal either with reflection (getClass, etc) because all types are not reified or with arrays because they are covariant at runtime.
The other case was dealing with the world before the generics were introduced, but it's pre-2004. 
Most people do not need unsafe casts in there code, so with optimism and faith to humankind, i think we should try to only allow __match and pattern matching if they are safe. The septic in me think we can always back off.  

Rémi

> 
> Maurizio
>>
>>> I think instanceof makes sense to describe what the generated code
>>> should do - but I believe applying the same type checking rules you have
>>> for instanceof would result in overly restrictive semantics.
>>>
>>> Maurizio
>> Rémi
>>
>>>> Rémi
>>>>
>>>> ----- Mail original -----
>>>>> De: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
>>>>> À: "Tagir Valeev" <amaembo at gmail.com>, "amber-dev" <amber-dev at openjdk.java.net>
>>>>> Envoyé: Mardi 3 Octobre 2017 10:12:11
>>>>> Objet: Re: [patterns] Unchecked cast warning is absent?
>>>>> I don't think you want to disallow. It's same as cast - you can do
>>>>>
>>>>> (T)obj
>>>>>
>>>>> Which is a no-op, and you get an unchecked warning for it. If T has some
>>>>> more interesting bound, the cast is not a no-op.
>>>>>
>>>>> Maurizio
>>>>>
>>>>>
>>>>> On 03/10/17 05:28, Tagir Valeev wrote:
>>>>>> It's even more interesting that method type parameter is allowed in __matches:
>>>>>>
>>>>>>      static <T> void matches(Object obj) {
>>>>>>        if(obj __matches T t) {
>>>>>>          System.out.println("Matches!");
>>>>>>        }
>>>>>>      }
>>>>>>
>>>>>> Bytecode shows that "instanceof Object" is generated which is pretty
>>>>>> useless. I think, such construct should be disabled and result in
>>>>>> compilation error (similarly to instanceof).
>>>>>>
>>>>>> With best regards,
>>>>>> Tagir Valeev.
>>>>>>
>>>>>> On Mon, Oct 2, 2017 at 4:45 PM, Tagir Valeev <amaembo at gmail.com> wrote:
>>>>>>> Hello!
>>>>>>>
>>>>>>> Just tried to play with pattern matching implementation. Seems that
>>>>>>> the matching against a non-reifiable type is unsafe, which is
>>>>>>> expected. Should the warning be displayed in this case?
>>>>>>>
>>>>>>> E.g.:
>>>>>>>
>>>>>>> import java.util.*;
>>>>>>>
>>>>>>> public class ListTest {
>>>>>>>
>>>>>>>      static void matches(Object obj) {
>>>>>>>        if(obj __matches List<String> list) {
>>>>>>>          System.out.println(list.get(0));
>>>>>>>        }
>>>>>>>      }
>>>>>>>
>>>>>>>      public static void main(String[] args) {
>>>>>>>        matches(Collections.singletonList(1));
>>>>>>>      }
>>>>>>> }
>>>>>>>
>>>>>>> $ ~/j/bin/javac -Xlint:all ListTest.java
>>>>>>> (compilation successful, no warning)
>>>>>>> $ ~/j/bin/java ListTest
>>>>>>> Exception in thread "main" java.lang.ClassCastException:
>>>>>>> java.base/java.lang.Integer cannot be cast to
>>>>>>> java.base/java.lang.String
>>>>>>> at ListTest.matches(ListTest.java:7)
>>>>>>> at ListTest.main(ListTest.java:12)
>>>>>>>
>>>>>>> With best regards,
> >>>>>> Tagir Valeev.