Feedback: Using pattern context to make type patterns more consistent and manage nulls

[Remi Forax]

----- Mail original -----
> De: "Stephen Colebourne" <scolebourne at joda.org>
> À: "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Lundi 25 Janvier 2021 01:46:49
> Objet: Re: Feedback: Using pattern context to make type patterns more consistent and manage nulls

> On Sun, 24 Jan 2021 at 16:56, Brian Goetz <brian.goetz at oracle.com> wrote:
>> I think the “OMG the nulls will eat us in our sleep” fears are dramatically
>> overblown.  Before we set anything on fire, I think we should come to terms
>> with why we think there is such a problem.
> 
> And I'll point out that my last email tried really hard to express
> that I consider null to be a secondary effect here. I'm not concerned
> with null, but with basic code readability and consistency.
> 
>> Type patterns do work consistently when you zoom out just a tiny bit, and
>> recognize that pattern matching exists in a static type context.  Suppose I
>> have `record Box(Object) {}`.  Then the pattern:
>>     case Box(Object o):
>> is, in some sense is “one” pattern match; the inner pattern (Object o) can be
>> statically determined to always succeed when the outer one does, and therefore
>> the only dynamic test here is for Box-hood.  So we test for Box-hood,
>> conditionally cast, extract the contents, and *assign* it to o.  Assignment
>> doesn’t say “if the contents are null, fail”; it’s just that there *is no inner
>> match here*.   But in the pattern:
>>     case Box(String s):
>> this is “two” pattern matches: even if the outer pattern matches, the inner
>> might not.  There are two dynamic tests, first for box-hood, and then for
>> string-hood of the box contents.
> 
> That is a huge inconsistency! A developer has nothing in the code to
> separate the static one from the dynamic one:
> 
> switch (box) {
>   case Box(Integer i) ...
>   case Box(Number n) ...
> }
> 
> Reading this code I have no way of knowing what it does. None.
> 
> If box is Box<Number> it does one thing. If box is Box<Object> it does
> something else. Sure the impact is only on null, but that is a
> secondary detail and not what is driving my concern. The key point is
> that someone reading the code can't tell what branch the code will
> take, and can get a different outcome for two identical patterns in
> different parts of the codebase.
> 
>> zoom out just a tiny bit, and recognize that pattern matching exists in a static
>> type context
> 
> This is the key bit. While obviously there is a static type context
> that provides a boundary to the problem space, the context can be
> *invisible*. What you are offering is a proposal where the meaning of
> the pattern `Type t` varies based on hidden information. Using the
> static context is effectively a premature optimization with very
> negative effects.
> 
> What I am suggesting is a simple requirement, that a pattern `Type t`
> is always dynamic, matching like instanceof wherever it is found, This
> has to be the requirement because that is what the developer
> physically wrote in the code, and identical looking coding elements
> should work in identical ways (eg. String s and Object o).
> 
> If you can find an alternative to using `var` in the way I propose
> that is fine by me. As I pointed out in my last email, the situations
> where there is a conflict to resolve are relatively rare, because best
> practice is to use `var` for the final case anyway.
> 
> As it stands, the proposal will never be acceptable to me because it
> fails the code readability test - premature optimization by using the
> static context means the code doesn't do what it says it does.

I'm sympathetic to what you are writing because it was what i was thinking few months ago.
I totally agree that 
  switch(box) {
    case Box(Integer i) ...
    case Box(var number) ...
  }
is more readable in term of intent.

But at the same time as a developer, i want to know what is the inferred type of "number", at least in my IDE, given that var is inferred as Number,
it's not unreasonable to think that if i replace var by Number, it works the same way.

Now, the discussion about null, i or number are bindings, more or less like multiple return parameters (Brian: i said more or less).
Those bindings will be sometimes null, by example,
  map.put("what!", null);
  ... map.value("what!", var value) -> ... // value is null here

so we (the EG) have to choose what is the best semantics, throw a NPE by default or being null permissive.
Obviously, as a user you don't want to propagate null so raising a NPE seems a good idea, but in fact it's not.
First, it means that in a middle of a switch, you will get spurious NPE but more importantly it's not compatible with the way we deal with null when we do a method call (invoking a pattern is alike doing a method call). With a method call, we will check if the return is null or not and act accordingly.

Another idea is to split things saying, the compiler should mandate a Box(null), but once you start to match multiple bindings, you go to combinatorial hell.

So saying that the total pattern is called if there is a null seems a good pragmatical choice.

> 
> Stephen

Rémi