Warn when Unit is used where () is expected

Compiler
1

Recently I’ve found out that Scala compiles the following code:

def weird : Either[Unit, Unit] = Right(Unit)

It seems that unit return inference kicks in and the code is rewritten to something like Right({ Unit; () }). This is actually the third time I’ve seen someone want to use Unit type and end up using Unit object instead of (). This example is particularly insidious, since reasonable changes will break it:

scala> def weird : Either[Unit, Unit] = {
     |   val res = Right(Unit)
     |   res
     | }
<console>:13: error: type mismatch;
 found   : scala.util.Right[Nothing,Unit.type]
 required: Either[Unit,Unit]
         res
         ^

If this is a problem others have encountered, could we tweak the compiler to by default emit a warning when Unit object is used where the Unit type is expected? The warning would recommend replacing Unit with ().

1 Like
2

You get a warning for this if you enable -Ywarn-value-discard:

$ scala -Ywarn-value-discard
scala> def f: Unit = Unit
<console>:11: warning: discarded non-Unit value
       def f: Unit = Unit
2 Likes
3

The intention between the warnings is different though: one is supposed to help beginners understand that they are doing something which they don’t want to do, while -Ywarn-value-discard will emit a warning everywhere a value is discarded, which I suspect makes a lot of people not use it.

Ideally, this kind of code would not compile at all, but I suspect it won’t be easy to change unit inference to uniformly achieve that. At least, I’m pretty sure I’ll be able to implement the proposed warning (which is part of why I’m proposing this), but changes to unit inference are a whole different game.

4

It’s especially good that it says “discarded non-Unit value” when it discards the value Unit.

A conversion from Unit.type => Unit doesn’t help because value discard is built in.

It would be nice if -Xfatal-warnings could turn the warning into a typer error; then maybe it would try the conversion (which could also warn). Otherwise, as they say out West with a mysterious squint, “linter territory.”

3 Likes
5

I’ve always wondered why value Unit is not the (only) value of type Unit. Then () would just be syntax sugar for scala.Unit, and we would never have this confusion.

6 Likes
6

Failing that, I can absolutely see something like Unit.value as a synonym for ().

My intuition when invoking a method with () as the sole parameter is that the parentheses would just collapse like they do in algebra, and I’m sure I can’t be alone here - it just doesn’t look at all right or parse well.

A way to express the concept that isn’t pure syntax would be a definite win.

7

I’m not sure what you mean, which warning are you referring to apart from value-discard? A hypothetical one?

Sure, I think this should be an error for several reasons: Why can I refer to Unit on the value level? Why do values automatically get converted into ()?

I would be all for making this a type error, that is the truth after all. Compare this with Haskell:

λ f :: () = 'x'

<interactive>:4:15: error:
    • Couldn't match expected type ‘()’ with actual type ‘Char’
8

@bergmark: Haskell has a touch different ecosystem & goals than Scala, so removing value discard completely is a no-go. I’ve thought a bit about adjusting rules to value discard. I think value discard could be restricted only to methods, functions and by-name parameters without any real breakage. This would fix the example in OP. However, there’s still potential for error: a method can be annotated as returning Unit and end in Unit expression. Similarly (() => Unit) : (() => Unit) is a function, so value discard would kick in, pretending that Unit is (). To be honest, the current behaviour is too much like an implicit conversion from Any to Unit for my liking, but at the same time changing it is not simple.

@LPTK: That’s actually a great point! Making Unit refer to () is doable with a bit of trickery. Unit on value level currently references Unit companion object. It contains box / unbox methods, so it’s necessary to keep it in some form. It should be really uncommon to actually need to reference that object though, so it could be moved together with Unit definition inside a new package (scala.unit perhaps?), and Predef could get two new definitions:

type Unit = scala.unit.Unit
val Unit = ()

The only issue is that this may be a non-trivial amount of work for relatively little gain. Compiler would likely need to be adjusted. Binary compatibility would certainly be broken, so this could only be included in 2.13. The fact that Unit does not reference the companion object of Unit is slightly confusing, but I’d say the change is worth it.

Long-term, I think I would be in favour of the solution above. However, it’s a fair bit more work than just emitting the warning I proposed originally, and it would need to wait until major version release. Given that, I still think adding a warning (with no flag necessary) when Unit object is discarded is a good makeshift fix. Do we agree on this? If we do, I’ll get to working on it.

9

Isn’t the solution, if any is really needed, just to remove the Unit companion object? Those 2 methods could (or should?) just as well be somewhere in the scala.runtime package.

2 Likes
10

To be perfectly honest, I have no idea if Unit object can be removed! If that’s possible to do, that’d be great. However, looking only as far as AnyValCompanion, we have:

package scala

/** A common supertype for companion classes of primitive types.
 *
 *  A common trait for /companion/ objects of primitive types comes handy
 *  when parameterizing code on types. For instance, the specialized
 *  annotation is passed a sequence of types on which to specialize:
 *  {{{
 *     class Tuple1[@specialized(Unit, Int, Double) T]
 *  }}}
 *
 */
private[scala] trait AnyValCompanion extends Specializable { }

Which means that Unit companion object is used as an argument to specialized annotation. This’d need to be adjusted if we rewired Unit to mean ().

In addition, I have no idea if the object isn’t accessed directly by the compiler to box/unbox (), or even if the notion of boxing/unboxing makes sense for Unit. Also, the fact that Unit will then be the sole exception to the rule “all primitive types have a companion object” is not entirely positive. Removing Unit object really does not seem like a small change to me, and I think we agree that the problem isn’t too big.

I’ve recounted, and out of 6 people at my workplace that tried to use () (myself included), 4 used Unit in some form instead (including one case where we ended up with Unit.type instead of Unit!). We may be an outlier, but I doubt it seeing the reactions of people to this kind of code compiling. The warning on Unit object discard would help, would take about ~10 LOC judging from the implementation of -Ywarn-value-discard, and I’m perfectly willing to do it. It could even continue to be useful after further changes to value discard: consider that if it was restricted to methods, functions and by-name parameters as I proposed, Unit : Unit would no longer compile, and following code would trigger a warning:

scala> (() => Unit) :  (() => Unit)
<console>:11: warning: discarded Unit object of type Unit.type. To create a value of type Unit, use ().
       (() => Unit) :  (() => Unit)
              ^

The problem isn’t too big, so the solution shouldn’t be too complex either. This’d be one puzzler less for the newcomers.

2 Likes
11

Som Snytt has submitted a PR on this: https://github.com/scala/scala/pull/6490

12

For clarity, that PR was opened as a result of https://github.com/scala/scala/pull/6465, which implements the warning I’ve proposed. So far it seems that we will go with the more direct option in https://github.com/scala/scala/pull/6490 and it will be impossible to reference Unit object in runtime code (@specialized will continue working).

1 Like
13

Also for the record, my PR belies Aleksander’s previous observation that the solution shouldn’t be too complex. I was inspired by a stray comment by Adriaan to hope that a puzzler may yet rise where another was slain.