[patterns] Unchecked cast warning is absent?

[Maurizio Cimadamore]

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

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.