switch: using an expicit type as total is dangerous

[forax at univ-mlv.fr]

----- Mail original -----
> De: "daniel smith" <daniel.smith at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Brian Goetz" <brian.goetz at oracle.com>, "amber-spec-experts" <amber-spec-experts at openjdk.java.net>
> Envoyé: Mardi 1 Septembre 2020 02:16:47
> Objet: Re: switch: using an expicit type as total is dangerous

>> On Aug 30, 2020, at 5:37 AM, forax at univ-mlv.fr wrote:
>> 
>> Ok, let's take an example, i've written a method getLiteral()
>>  Number getLiteral(String token) {
>>    if (token.equals("null")) {
>>      return null; // null is part of the domain
>>    }
>>    try {
>>      return Integer.parseInt(token);
>>    } catch(NumberFormatException e) {
>>      return Double.parseDouble(token);
>>    }
>>  }
>> 
>> and a statement switch in another package/module
>>  switch(getLiteral(token)) {
>>    case Integer -> System.out.println("Integer");
>>    case Double -> System.out.println("Double");
>>    case Number -> System.out.println("null");
>>  }
>> 
>> but now i change getLiteral() to add string literal
>>  Object getLiteral(String token) {
>>    if (token.equals("null")) {
>>      return null; // null is part of the domain
>>    }
>>    if (token.startsWith("\"") {
>>      return token.substring(1, token.length() - 1);
>>    }
>>    try {
>>      return Integer.parseInt(token);
>>    } catch(NumberFormatException e) {
>>      return Double.parseDouble(token);
>>    }
>>  }
>> 
>> If i only recompile getLiteral(), and run the code containing the switch, i get
>> a ICCE at runtime because the signature of getLiteral() has changed, which is
>> good,
>> but if i now recompile the switch, the code compiles without any error but with
>> a different semantics, duh ?
> 
> One thing to notice is that, depending on context, the real bug report coming
> out of this sequence of events is quite likely to have the form "The
> Widget.poke method doesn't behave correctly when operating on a string." The
> subtle difference in the treatment of 'null' may be relatively obscure.

It is obscure because null is usually not part of the domain so it will take a long time to debug this kind of issue.

> 
> Anyway, what I think you're really after is a way for the programmer to assert
> that silently falling out of this switch is unexpected. And 'sealed switch'
> (however expressed syntactically) is the tool you need to do that. (Let's not
> dive into aspects of that feature here, though, there's another thread.)

yes, it's one solution, the other is to always use 'var' when what you want is a total pattern. 

> 
> Another thing you may appreciate is a warning or error if a non-sealed switch
> has a total case, because if the switch is total, it's very likely expected to
> be total forever. We could do this with optimistically-total enum switches,
> too. I don't think I'd want to do it with 'default' switches, which can be
> thought of as the "legacy" version of 'sealed switch'.

It's more or less what a part of what i've proposed in another thread,
i propose to emit a warning even if there is a default.
Combined with the fact that you can declare a total switch means that you can suggest to users to retrofit the existing enum switch (and uses of the new switch on types) to a total switch.

> 
> (Whether this is a code-style-enforcing 'lint' warning or a compiler error
> probably comes down to a cost-benefit analysis. "How likely is the switch
> operand type to change?" vs. "How annoyed will programmers be that we're making
> them add 'sealed' or rewrite to 'default'?")

If IDEs provide this an automatic refactoring, the cost of changing the code may be greatly reduced.

Rémi