Skip to content

‘if … else if’ expression unexpectedly has Unit value when last branch taken #14914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
djneades opened this issue Apr 12, 2022 · 13 comments · Fixed by #14960
Closed

‘if … else if’ expression unexpectedly has Unit value when last branch taken #14914

djneades opened this issue Apr 12, 2022 · 13 comments · Fixed by #14960

Comments

@djneades
Copy link

djneades commented Apr 12, 2022

Compiler version

3.1.1

Minimized code

This code prints (), as a evaluates to the Unit value:

 val a =
    if false then
       1
    else if true then
       2
 
 println(a)

Update: this code may be a bit too minimized to demonstrate the scope of the actual problem. See the examples in this comment for more clarity.

Output

()

Expectation

Although this code is unusual and unlikely to be written by an experienced programmer (it is a simplified and minimized version of something written by my 8-year-old son who has just started learning to program with Scala), I suspect most people would intuitively expect a to evaluate to 2 rather than to ().

Adding an unconditional else clause to the if statement restores expected behaviour:

val a =
    if false then
       1
    else if true then
       2
    else
       3
 
 println(a) // prints 2
@djneades djneades added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 12, 2022
@som-snytt
Copy link
Contributor

I'm aware that this is as designed. Scala 2 would append else () if that leg is omitted. Scala 3 just takes the missing leg to mean "this is side-effecting".

Another context was whether Scala 2 -Wnon-unit-statement should warn in one-legged if, which is inherently side-effecting, or whether to warn about the if itself.

Turning it around, maybe the wart is consuming an else-less if.

You're never too young to learn to minimize Scala.

@som-snytt
Copy link
Contributor

Duplicates #12890

@KacperFKorban KacperFKorban added area:parser and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 13, 2022
@djneades
Copy link
Author

djneades commented Apr 13, 2022

@som-snytt Thank you for the comments and link to the related issue. The discussion on #12890 is specifically about the case of an if with a single branch (this ‘is essentially treated as a side-effectful statement’). The problem here is with 2 branches, and the behaviour in fact manifests with any number of branches, as far as I can tell.

If this (frankly weird and counter-intuitive) behaviour is really intended even with multiple branches, could we at least improve the warning message? I didn’t find the existing message (A pure expression does nothing in statement position; you may be omitting necessary parentheses) to be at all informative in identifying or correcting the problem.

Thank you for your consideration.

@som-snytt
Copy link
Contributor

Your question did make me think that an alternative behavior would be to pick a result type based on the existing legs, then add the "default" leg using the "default value" for the type, which is null.asInstanceOf[T]. However, the direction is now always to make null explicit. There is a better chance of seeing a lint for side-effecting if. For simple cases where there are no branches uncovered by predicates, it could also simplify. Thanks for bringing up the issue. Personally, I don't use branches and booleans any more, because I always wind up tails when it should be heads.

@djneades
Copy link
Author

@som-snytt :-)

The situation is perhaps more subtle than may have thus far been realized, which is why it is so confusing (and it took me a while to figure out what was going on in my son’s code). It is not simply the case that these if expressions always have a Unit value. They only have a Unit value if the last branch of the if is taken; otherwise, they have the expected value.

Thus:

val x = "1"

val a =
    if x == "1" then
       1
    else if x == "2" then
       2
    else if x == "3" then
       3
 
 println(a) // prints 1

I’m not even seeing a compiler warning with the above code to give me a hint that there is a problem (at least, not on Scastie).

And, likewise:

val x = "2"

val a =
    if x == "1" then
       1
    else if x == "2" then
       2
    else if x == "3" then
       3
 
 println(a) // prints 2

Again, there is no compiler warning for the above.

However:

val x = "3"

val a =
    if x == "1" then
       1
    else if x == "2" then
       2
    else if x == "3" then
       3
 
 println(a) // prints ()

At least this one gives the compiler warning.

I am struggling to see a justification (from a Scala user’s perspective) for why the first two examples here yield a sane (i.e. non-Unit) value for a, but the last does not.

@som-snytt
Copy link
Contributor

Oh my, you were right that it is just one-legged if that is special-cased for value discard. I would guess the AnyVal inferred just the same as Scala 2. Sorry for my mistake. Once I'm on my second if, I switch to match (or switch in Java).

scala> def f(x: String) = {
     |   if x == "1" then 1
     |   else if x == "2" then 2
     |   else if x == "3" then 3
     | }
1 warning found
def f(x: String): AnyVal
-- [E129] Potential Issue Warning: ------------------------------------------------------------------------------------------------------
4 |  else if x == "3" then 3
  |                        ^
  |                        A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`

scala> def f(x: String) = {
     |   if x == "1" then 1
     | }
1 warning found
def f(x: String): Unit
-- [E129] Potential Issue Warning: ------------------------------------------------------------------------------------------------------
2 |  if x == "1" then 1
  |                   ^
  |                   A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`

@djneades
Copy link
Author

(Note that I’d be fine with the if yielding () if none of the branches were taken.)

@som-snytt
Copy link
Contributor

som-snytt commented Apr 13, 2022

That's pretty funky. Just the penultimate branch (before the missing else) gets its value discarded (as I now see you said). That must be a glitch.

➜  dotty git:(test/current) ./bin/scala -Vprint:parser,typer
Welcome to Scala 3.1.3-RC1-bin-SNAPSHOT-git-f3cca47 (17.0.2, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> def f(x: String) = {
     |   if x == "1" then 1
     | }
[[syntax trees at end of                     typer]] // rs$line$1
package <empty> {
  final lazy module val rs$line$1: rs$line$1 = new rs$line$1()
  final module class rs$line$1() extends Object() { this: rs$line$1.type =>
    def f(x: String): Unit =
      {
        if x.==("1") then
          {
            1
            ()
          }
         else ()
      }
  }
}

1 warning found
def f(x: String): Unit
-- [E129] Potential Issue Warning: ------------------------------------------------------------------------------------------------------
2 |  if x == "1" then 1
  |                   ^
  |                   A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`

scala> def f(x: String) = {
     |   if x == "1" then 1
     |   else if x == "2" then 2
     |   else if x == "3" then 3
     | }
[[syntax trees at end of                     typer]] // rs$line$2
package <empty> {
  final lazy module val rs$line$2: rs$line$2 = new rs$line$2()
  final module class rs$line$2() extends Object() { this: rs$line$2.type =>
    def f(x: String): AnyVal =
      {
        if x.==("1") then 1 else
          if x.==("2") then 2 else
            if x.==("3") then
              {
                3
                ()
              }
             else ()
      }
  }
}

1 warning found
def f(x: String): AnyVal
-- [E129] Potential Issue Warning: ------------------------------------------------------------------------------------------------------
4 |  else if x == "3" then 3
  |                        ^
  |                        A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`

scala>

@djneades
Copy link
Author

@som-snytt Like you, I would use a match here. My son did know about match at the time he wrote his code, but chose to use the multi-branched if anyway (he was just playing on his own). His little program prompted the user for a couple of numbers and an arithmetic operation (+, /, -, *). It was working fine for three of the operations, but he was puzzled (and so was I!) as to why he was seeing () output for the last operation.

@djneades
Copy link
Author

@som-snytt:

That's pretty funky. Just the penultimate branch gets its value discarded. That must be a glitch.

Agreed! Could we re-open this issue then, please?

@djneades djneades changed the title ‘if … else if’ expression unexpectedly has Unit value ‘if … else if’ expression unexpectedly has Unit value when last branch taken Apr 13, 2022
@som-snytt
Copy link
Contributor

@KacperFKorban hi, if you agree, would you mind reopening? My previous judgment may have been premature. Or, we could start a discussion in discussions.

@som-snytt
Copy link
Contributor

@djneades That's a great story. You're never too young to learn to distrust computers.

@KacperFKorban KacperFKorban reopened this Apr 13, 2022
@djneades
Copy link
Author

@som-snytt:

@djneades That's a great story. You're never too young to learn to distrust computers.

Indeed! It’s a good lesson to learn that the compiler can be wrong (or, at least, weird). Ideally, though, that lesson would have come a little later in his programming experience :-)

odersky added a commit to dotty-staging/dotty that referenced this issue Apr 18, 2022
Previously only a one-armed if was treated as a statement
where all parts were typed with Unit as expected type. We now
extend that treatment also to multi-branch ifs that lack
a final part.

Fixes scala#14914
michelou pushed a commit to michelou/scala3 that referenced this issue Apr 25, 2022
Previously only a one-armed if was treated as a statement
where all parts were typed with Unit as expected type. We now
extend that treatment also to multi-branch ifs that lack
a final part.

Fixes scala#14914
@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants