Compiler warnings on discarded expressions?

[Adam Warski]

Hello,

the compiler flag -Ywarn-value-discard (+ fatal warnings) is very helpful in avoiding quite subtle bugs which can occur due to the to-unit value coercion (a "classical" example in my case is doing a `map` instead of a `flatMap` when the result is `Future[Unit]`).

However, what about discarded expressions/values? E.g. this causes a warning:

scala> def g(): Int = 42

g: ()Int

scala> def h(): Unit = { g() }

<console>:8: warning: discarded non-Unit value

       def h(): Unit = { g() }

                          ^

h: ()Unit

But this doesn't:

scala> def h(): Unit = { g(); () }

h: ()Unit

Especially when doing more "functional"-style Scala, when most of the methods are pure, a discarded expression or value is usually a bug.

A less abstract example of what *might* be a bug is discarding a created future:

def f(): String = {

  Future { /* computation */ }

  "ok"

}

While this might be perfectly ok, I would say that usually in "production" code you would want to do something with the future, even if it's logging a potential error in onComplete.

When moving to more "functional" structures, such as scalaz/fs2's Task, this gets even worse:

def f(): String {

  Task { /* computation */ }

  "ok"

}

Now this definitely is a bug - there's no point in creating a Task which is never run. And so on ...

I expected this to be a well-discussed problem, but I only found a relatively unpopular SO question (http://stackoverflow.com/questions/17691487/scala-does-not-warn-about-unused-computation-or-value) and a half-finished compiler plugin created by a friend which performs such a check. 

Question is: is adding discarded expression warnings a fundamentally bad idea? Maybe it would cause a flood of warnings in regular Scala code (though I don't think I see this happening)? 

Or should I get to work and finish the plugin? ;)

-- 

Adam Warski

[Jed Wesley-Smith]

yeah, the warning is very inconsistent and doesn't catch a number of important cases. It is better than nothing, but not as good as it could be.

--
You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[som-snytt]

Someone has suggested an annotation for types of values that must be consumed, such as you describe.

Current thinking is that warnings belong in a tool like scala-abide. The reason is that they are difficult to tune just right. (That makes them a maintenance issue and also tied to the compiler release cycle.)

For example, someone asked that -Ywarn-value-discard not warn for the following:

def add(x: X): Unit = listBuffer += x

Otherwise, that particular warning is very narrowly scoped and therefore consistent.

The following warns ordinarily, when it can feebly detect a pure expression:

scala> def f(): Unit = { 42 ;() }
<console>:11: warning: a pure expression does nothing in statement position
def f(): Unit = { 42 ;() }
^
f: ()Unit

I have a PR for improving -Ywarn-unused so you can tell it what to warn about (imports, params, implicits, etc):

https://github.com/scala/scala/pull/5402/files

Under that regime (where you can turn it off), it's not unreasonable to -Ywarn-unused:value. Maybe create a ticket for it.

I have not heard if they want to accept that PR or push all warning out to Abide.

[Adriaan Moors]

Until abide takes off, we will take PRs in scala/scala for improvements of these compiler warnings. One might say that as long as we take such PRs, abide won't gain traction, but I think that's selling it short (I mean, with a name like that, how can you resist?).

--

You received this message because you are subscribed to the Google Groups "scala-user" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+...@googlegroups.com.

[Oliver Ruebenacker]

     Hello,

  What kind of reasoning would the compiler use to know that creating a Task without doing anything with it is a a bad thing?

     Best, Oliver

--

You received this message because you are subscribed to the Google Groups "scala-user" group.

To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Oliver Ruebenacker

Senior Software Engineer, Diabetes Portal, Broad Institute

[Oliver Ruebenacker]

     Hello,

  Two or three years ago, I was pointed to some Scala library as proof that you could do GUIs in functional style. Unfortunately, I forgot how it was called and have not been able to find it quickly on Google.

  In that library, if you wanted some GUI element to appear, you just created an object that represented that element. No need to do anything with that object.

     Best, Oliver

On Fri, Dec 2, 2016 at 3:07 PM, Adam Warski <ad...@warski.org> wrote:

--

You received this message because you are subscribed to the Google Groups "scala-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Adam Warski]

Thanks for the pointer to abide! This looks more like a meta-plugin which can be customised using specific rules, so maybe one can create rules to detect discarded expressions - I'll take a look into that.

But having warnings separated from the compiler definitely makes sense - esp if it would be an "offical" tool.

I can definitely see some situations where you would want the value-discard warning to be disabled, but that could be probably tuned with an annotation? Or maybe the abide rules could be created only so that "known" pure values (e.g. probably everything in scalaz.* or cats.*) would trigger warnings.

Adam

[Adam Warski]

In general it's not possible for the compiler to know that. It could make good guesses ... but as I wrote in another post, it would probably make sense to selectively enable or disable the value-discard warning.

The choice is of course what should be the default - enabled or disabled; and that might differ per-project. A project written in FP-mostly style would choose a default "enabled". A project which uses a lot of mutables values would probably by default disable the warning.

Adam

To unsubscribe from this group and stop receiving emails from it, send an email to scala-user+...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[mikael....@magine.com]

Such warning would be useful, but then we need a way to explicitly discard a value/expression and avoid the warning.

Currently { g(); () } is a way to explicitly discard and avoid the warning.