Introduce Unit.void[A](x: A): Unit #7530

dwijnand · 2018-12-14T17:48:19Z

This is useful when you want to explicitly discard a value, without
having to val it and force your expression to be a block.

Ideally, this would be defined on the Unit type rather than the Unit
companion object, but I couldn't figure out the backend changes
necessary to get it to handle that...

Possible alternative names: discard, drop, forget, unit, ...

TODO:

Finalise the name
Finalise the location
Fix the test so it absolutely tests

Review (and help wanted) by @lrytz or @adriaanm, kindly.

This is useful when you want to explicitly discard a value, without having to val it and force your expression to be a block. Ideally this would be defined on the Unit type rather than the Unit companion object, but I couldn't figure out the backend changes necessary to get it to handle that...

Ichoran · 2018-12-14T18:30:34Z

What is wrong with { x; () } instead of Unit.void(x)?

dwijnand · 2018-12-14T18:49:32Z

I'd prefer having a method, and not need a block.

The best way is having it as a method on Any (like toUnit), which we can do if it's preferred.

src/library/scala/Unit.scala

som-snytt · 2018-12-14T22:34:31Z

This proposal will run afoul of disallowing references to Unit companion because folks confuse it with the unit value.

I already made a similar joke, but actually why not co-opt asInstanceOf.

dwijnand · 2018-12-14T23:09:56Z

This proposal will run afoul of disallowing references to Unit companion because folks confuse it with the unit value.

I agree Unit should be its own companion object, rather than have one. But I'm ignoring that for now and just doing what's easier to implement.

I already made a similar joke, but actually why not co-opt asInstanceOf.

If you mean co-opt in the standard library, then co-opt away. But in user-land I'd like a function somewhere.

dwijnand · 2018-12-14T23:13:32Z

What is wrong with { x; () } instead of Unit.void(x)?

I forgot another good reason, @Ichoran: to make it explicit that you're discarding a value. Obviously, your example is pretty obvious (and made me forget), but it would be nice to have the option of being explicit.

Also: that doesn't work for the use case that I noticed in a project that has a local version of this:

class StringContextOps private[skunk](sc: StringContext) {
  void(sc)

  def sql(argSeq: Any*): Any =
    macro StringContextOps.StringOpsMacros.sql_impl

  def id(): Identifier =
    macro StringContextOps.StringOpsMacros.identifier_impl

}

som-snytt · 2018-12-14T23:26:16Z

The explicit version is { x ;() }, a winking laughing face.

The joke on the other ticket was to use sc.asInstanceOf[Unit] to silence unused warnings.

Ichoran · 2018-12-15T04:47:37Z

@dwijnand - What is the advantage of a method here over a block? It's just another way to do things. It's not like { foo(); () } is mysterious, is it? The method alone isn't sufficient documentation for what is going on. I guess your best bet, that way, for the method name is from. Unit from "salmon" makes it pretty clear what you should expect--a ()--even though it's a bit odd that you would need it "from" anything.

(Also, I don't know what void(sc) is supposed to do that { sc; () } doesn't do.)

dwijnand · 2018-12-15T07:13:52Z

The advantage is it's a function, which has a name. It's not just a jumble of symbols (2 brackets, 2 parens and a semicolon). It's an express decision to transform a value to unit.

The method alone isn't sufficient documentation for what is going on.

I disagree. (Unless you mean "void isn't documented", in that case, fair point)

dwijnand · 2018-12-15T07:17:23Z

(Also, I don't know what void(sc) is supposed to do that { sc; () } doesn't do.)

Well, I guess you could've done that. But I didn't even think you could, because I'm already in a block (the class constructor). In retrospect it's obvious. But it would also be unnecessary if we just had... a function available in the standard library that allowed me to discard a value, without having to do the secret symbol dance of { sc; () }.

dwijnand · 2018-12-15T07:44:02Z

Actually { sc; () } doesn't work because sc goes unused:

[error] /d/skunk/modules/core/src/main/scala/syntax/StringContextOps.scala:13:5: a pure expression does nothing in statement position
[error]   { sc; () }
[error]     ^
[error] one error found

so then you try { val _ = sc; () } but that doesn't work either:

[error] /d/skunk/modules/core/src/main/scala/syntax/StringContextOps.scala:13:3: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
[error]   { val _ = sc; () }
[error]   ^

So then you just go back to using (and therefore locally defining) void... 🙄

dwijnand · 2018-12-15T07:44:50Z

val _ = sc works, but I'd rather use the more expressive void(sc)

Ichoran · 2018-12-15T07:51:28Z

@dwijnand - I mean if I see Unit.void(m.replace(k, v)), I have no intuition for what void does. I can learn, of course--I can go read the docs, or I can think for a while about why a Unit.void might exist, and possibly stumble into the idea that it's generating (). But it's unfriendly.

Unit.discard(m.replace(k, v)) is somewhat better, but it's rather puzzling why Unit is the one discarding the expression. Why not just discard(m.replace(k, v))? And anyway, if you discard it but everything is an expression, what's left? null and () are two possible choices, and it's reasonable to surmise that perhaps it's Unit.discard(m.replace(k,v)) instead of Null.discard(m.replace(k, v)) because it's unit?

In contrast, { m.replace(k, v); () } makes it really clear what is happening. The problem of course is that it's less clear why it's happening, but at least you know what.

The only method name that might possibly have this property which isn't a full sentence is from. Then it's really clear, also, that you're generating a (). I'm still not convinced it's better than { ; () }, but at least it tells you what is actually being returned instead of what your goals were when you used it.

Also, with your example, why does either void(sc) or { sc; () } actually do anything? sc isn't a by-name parameter. The whole thing seems pointless, unless it's to avoid some other implementation detail that you also don't want to care about.

test/junit/scala/UnitTest.scala

viktorklang · 2018-12-15T09:06:54Z

Unit.eval? -- Cheers, √

adriaanm · 2018-12-15T14:09:41Z

How about a type alias `type Unused = Unit @unchecked`? And then `sc(): Unused`

…

On Sat, Dec 15, 2018 at 11:01 kerr ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/junit/scala/UnitTest.scala <#7530 (comment)>: > @@ -0,0 +1,8 @@ +package scala + +import org.junit.Assert._ +import org.junit.Test + +final class UnitTest { not test no unused warning too — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7530 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjy4m-EA0NKOesOHJKTnCLMD18xIhXks5u5MiIgaJpZM4ZT8rj> .

dwijnand · 2018-12-15T14:26:13Z

@Ichoran

@dwijnand - I mean if I see Unit.void(m.replace(k, v)), I have no intuition for what void does. I can learn, of course--I can go read the docs, or I can think for a while about why a Unit.void might exist, and possibly stumble into the idea that it's generating (). But it's unfriendly.

I don't think these are strong arguments. It's hard to prove intuition and friendliness I think. I think it's partially based on what you know. Clearly, you don't know Unit.void because it's never existed before as that, and I'm up for finding a better location and name for it, but I don't think it's a difficult concept to teach, and I agree with you that should include why it exists (TODO for me).

Unit.discard(m.replace(k, v)) is somewhat better, but it's rather puzzling why Unit is the one discarding the expression. Why not just discard(m.replace(k, v))?

You're right, and I'm looking for ideas:

I don't like @hepin1989's Unit.point idea
I don't think Unit.unit is a strong candidate
I don't understand @viktorklang's Unit.eval idea - Viktor, what's the reasoning for that name?
While Unit.discard feels somewhat better, it looks like we're discarding a unit, which is backwards...
discard in Predef would mirror identity but I'm unsure we should add to Predef
Given the target of the function is any type, Any#discard() would follow the convention and feels great when you use it, but I don't think we should add to Any either.

And anyway, if you discard it but everything is an expression, what's left? null and () are two possible choices, and it's reasonable to surmise that perhaps it's Unit.discard(m.replace(k,v)) instead of Null.discard(m.replace(k, v)) because it's unit?

Asides from not everything is an expression (there are also imports and definitions, TIL, SLS 6.25), everything is an expression but also we discard non-unit values and insert units in its place (which sometimes means no bytecode, sometimes means boxed unit).

In contrast, { m.replace(k, v); () } makes it really clear what is happening. The problem of course is that it's less clear why it's happening, but at least you know what.

Again, here it's clear to you what's happened because of what you know. To someone else it might not be clear what's happening to that value replace returned. (just for the sake of argument, one could go on and say the dot notation is unclear, the semi-colon is unclear, it's all relative and therefore a difficult argument to stand on IMHO)

The only method name that might possibly have this property which isn't a full sentence is from. Then it's really clear, also, that you're generating a (). I'm still not convinced it's better than { ; () }, but at least it tells you what is actually being returned instead of what your goals were when you used it.

I don't hate Unit.from. The other idea I came up with is on the Function object - it could sit next to its brother const.

Also, with your example, why does either void(sc) or { sc; () } actually do anything? sc isn't a by-name parameter. The whole thing seems pointless, unless it's to avoid some other implementation detail that you also don't want to care about.

According to that compilation unit that sc is completely unused, so it warns (or errors if you fatal warning). So I guess you could argue one could teach Xlint to not warn on constructor params when a class contains macros, but I think I'd rather silence it explicitly. One way is with this as a better expressing (IMHO!) alternative to val _ = sc, or with @deprecated("unused", "")/@unused (#7529) but that's weird because it's not actually unused... 😕

dwijnand · 2018-12-15T14:26:17Z

@adriaanm

How about a type alias type Unused = Unit @unchecked? And then sc(): Unused

Interesting idea! But it doesn't work 😕

[error] /d/skunk/modules/core/src/main/scala/syntax/StringContextOps.scala:15:3: type mismatch;
[error]  found   : StringContext
[error]  required: StringContextOps.this.Unused
[error]     (which expands to)  Unit @unchecked
[error]   sc: Unused
[error]   ^

viktorklang · 2018-12-15T15:00:48Z

Scrap the eval name—the thinking was that it takes an expression and makes it evaluate to Unit. Another idea is to call it ”of” so: Unit.of(expr) or have a toUnit on Any. -- Cheers, √

dwijnand · 2018-12-15T15:10:10Z

Another idea is to call it ”of” so: Unit.of(expr) or have a toUnit on AnyVal.

Don't mind Unit.of(expr) either.

Why toUnit on AnyVal? Assuming you mean AnyVal type and that uses this, that wouldn't make it available for any reference types, which should be able to be discarded.

viktorklang · 2018-12-15T19:49:53Z

@dwijnand Sorry, meant Any! :)

adriaanm · 2018-12-15T22:12:45Z

But we have the means to make it work! ;-)

…

On Sat, Dec 15, 2018 at 20:49 Viktor Klang (√) ***@***.***> wrote: @dwijnand <https://github.com/dwijnand> Sorry, meant Any! :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7530 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjyyfbb9MNr-G18Ilk3QQ2L3M3O0WZks5u5VJigaJpZM4ZT8rj> .

dwijnand · 2018-12-15T23:45:38Z

True.

Now I don't hate the idea, but why do you propose it over a method? For instance, as a comparison, why do Predef.identity and Function.const get exist, but not void?

He-Pin · 2018-12-16T04:41:00Z

True.

Now I don't hate the idea, but why do you propose it over a method? For instance, as a comparison, why do Predef.identity and Function.const get exist, but not void?

Shouldn't deprecate the const and introduce identity?

I think Function.ignore will be good.

NthPortal · 2018-12-16T06:51:22Z

We could add an apply method to Unit (it would be synthetic, obviously), so that you could call
()(value) 😉

In all seriousness, discard seems like the best name to me, but I'm also not convinced of its value.

joshlemer · 2018-12-16T07:19:02Z

Respectfully, I think that with the goals in mind of:

trying to limit ways in which there are needlessly multiple ways of doing something
simplifying the language and syntax
keeping the standard library small

I would prefer to this not be added. I think that when you want to explicitly discard an expression e and return (), the language itself provides a good mechanism for this like @Ichoran says: e; ()

Maybe if I saw some really nasty examples where having to do this hurts readability, I could be convinced though.

viktorklang · 2018-12-16T07:50:43Z

I agree. I think ”; ()” is enough. That said, the discoverability is suffering, so how about adding it to the compiler error message when Unit is expected but something else is provided? -- Cheers, √

He-Pin · 2018-12-16T10:01:19Z

The last () is not needed.

object Function {
  type Ignored = Unit
  def ignore[T](t: T): Ignored = {
    val _ = t
  }
}

dwijnand · 2018-12-16T13:29:24Z

True, the compiler can insert that unit for you.

som-snytt · 2018-12-16T14:58:39Z

Note that as of recently you can use the underscore trick multiple times in scope. M5 errors with multiple defs, which was one motivation for a macro that silently consumes a value.

scala> locally[Unit] { val x,y = 42 }
                           ^
       warning: local val x in value res0 is never used
                             ^
       warning: local val y in value res0 is never used

scala> locally[Unit] { val x,y = 42; val _ = x; val _ = y }

scala 2.13.0-M5> class C { private val x,y = 42; private[this] val _ = x; private[this] val _ = y }
                                                                                            ^
                 error: _ is already defined as value _

scala 2.13.0-M5> locally[Unit] { val x,y = 42; val _ = x; val _ = y }
                                                              ^
                 error: _ is already defined as value _

Maybe the pure expression does nothing in statement position warning could be nuanced to noticed the { x ; () } idiom, and obviate underscores. Maybe notice { x ; y ; () }.

tarsa · 2018-12-16T15:05:39Z

IIUC simple type ascription should solve the problem:

        // all constructs below compile fine under Scala 2.12.1
	5: Unit
	"dfs": Unit
	(5 + 5): Unit
	Option(5: Unit)

tarsa · 2018-12-16T17:29:53Z

I saw two examples in this thread.

One is with unused constructor parameter. We can turn it into private val, so it will always be a field in the bytecode.

Another one is about changing return type to Unit. Instead of doing { val _ = m.replace(k, v) } we can do m.replace(k, v): Unit.

WDYT?

adriaanm · 2018-12-16T19:25:46Z

I prefer a type ascription because you’re communicating with the type checker/linter, not the runtime.

…

On Sun, Dec 16, 2018 at 18:29 Piotr Tarsa ***@***.***> wrote: I saw two examples in this thread. One is with unused constructor parameter. We can turn it into private val, so it will always be a field in the bytecode. Another one is about changing return type to Unit. Instead of doing { val _ = m.replace(k, v) } we can do m.replace(k, v): Unit. WDYT? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7530 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjywGTOjhAloWXi35iM1TfLGZYeNP0ks5u5oMSgaJpZM4ZT8rj> .

dwijnand · 2018-12-16T21:04:22Z

I'd be fine with that, provided we can implement that in time...

OlegYch · 2018-12-16T23:34:48Z

'ignore' is the name i use for this function
it has to be a function since you can't attach docs to syntax tricks and because syntax tricks, well, interfere with syntax

lrytz · 2018-12-17T06:24:39Z

This PR LGTM in its current form. Other alternatives work as well.

src/library/scala/Unit.scala

tarsa · 2018-12-17T10:52:08Z

What about:

object Unit {
  def cast(ignored: Unit): Unit = ()
}

?
Simpler and immediately obvious the value is ignored.

dwijnand · 2018-12-17T10:59:16Z

@tarsa That doesn't work because the value would be ignored at the call-site, which is part of the whole point of this. Also cast implies something different, something that this function isn't doing.

I encourage you to try your ideas in the scala -Xlint -Ywarn-value-discard REPL.

Ichoran · 2018-12-17T16:35:45Z

If we're going to do this, ergonomically it would usually be more convenient as

implicit class AnythingCanBeDiscarded[A](private val thing: A) extends AnyVal {
  def discard: Unit = ()
}

dwijnand · 2018-12-17T16:44:47Z

I'd be happy to provide that, ala import scala.util.chaining._.

Perhaps import scala.util.discard._, or discarding? valueDiscard?

NthPortal · 2018-12-18T05:31:10Z

for reference on what is considered discarding a value:

scala> def foo(x: String): Unit = { x.length; () }
foo: (x: String)Unit

scala> def foo(x: String): Unit = { val _ = x.length }
foo: (x: String)Unit

scala> def foo(x: String): Unit = { x.length }
<console>:11: warning: discarded non-Unit value
       def foo(x: String): Unit = { x.length }
                                      ^
foo: (x: String)Unit

adriaanm · 2018-12-20T15:47:13Z

why do you propose it [a type ascription] over a method? For instance, as a comparison, why do Predef.identity and Function.const get exist, but not void?

You can't express those with a type ascription, and they actually have a run time component (i.e., you want to evaluate the function). In this case, we're just communicating intent / silencing warnings. I'm not in favor of adding a method for this.

dwijnand · 2018-12-20T17:38:17Z

I think just like () is a type, but doesn't have a runtime component, A => () is a perfectly valid function, so I don't see why it should be shunned from the standard library. Fair enough it can be elided from the bytecode like () is.

I'd actually be fine with your idea, it's just harder to implement and I don't know if I have the chops to do it in time...

adriaanm · 2018-12-20T19:05:44Z

I don’t think my approach needs to land in 2.13.0. We can do a hangout and pair program it tomorrow if you like, or next year :-)

…

On Thu, 20 Dec 2018 at 18:38, Dale Wijnand ***@***.***> wrote: I think just like () is a type, but doesn't have a runtime component, A => () is a perfectly valid function, so I don't see why it should be shunned from the standard library. Fair enough it can be elided from the bytecode like () is. I'd actually be fine with your idea, it's just harder to implement and I don't know if I have the chops to do it in time... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7530 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjy_rEU_792IvUonrV4Z1_RLwWkNiMks5u68sKgaJpZM4ZT8rj> .

dwijnand · 2018-12-21T11:15:38Z

Closed in favour of #7560.

scala-jenkins added this to the 2.13.0-RC1 milestone Dec 14, 2018

joshlemer added the release-notes worth highlighting in next release notes label Dec 14, 2018

He-Pin reviewed Dec 14, 2018

View reviewed changes

src/library/scala/Unit.scala Outdated Show resolved Hide resolved

martin-g reviewed Dec 15, 2018

View reviewed changes

test/junit/scala/UnitTest.scala Show resolved Hide resolved

viktorklang reviewed Dec 17, 2018

View reviewed changes

src/library/scala/Unit.scala Outdated Show resolved Hide resolved

Fix UnitTest testVoid

52ad2df

Mark Unit.void as final

a2529c7

dwijnand closed this Dec 21, 2018

dwijnand deleted the void branch December 21, 2018 11:15

SethTisue removed this from the 2.13.0-RC1 milestone Jan 3, 2019

SethTisue removed the release-notes worth highlighting in next release notes label Jan 3, 2019

dwijnand mentioned this pull request Feb 4, 2019

Strict mode for value discard warning #7716

Closed

dwijnand restored the void branch March 18, 2019 10:52

dwijnand deleted the void branch August 17, 2019 15:35

dwijnand mentioned this pull request Aug 12, 2020

Predef.unit to workaround "a pure expression does nothing" #9164

Closed

eed3si9n mentioned this pull request Aug 12, 2020

spurious "a pure expression does nothing" warning (awkward to suppress in 2.12, prone to occur in sbt builds) scala/bug#12112

Open

Introduce Unit.void[A](x: A): Unit #7530

Introduce Unit.void[A](x: A): Unit #7530

Conversation

dwijnand commented Dec 14, 2018 • edited

Ichoran commented Dec 14, 2018

dwijnand commented Dec 14, 2018

som-snytt commented Dec 14, 2018

dwijnand commented Dec 14, 2018

dwijnand commented Dec 14, 2018

som-snytt commented Dec 14, 2018

Ichoran commented Dec 15, 2018 • edited

dwijnand commented Dec 15, 2018

dwijnand commented Dec 15, 2018

dwijnand commented Dec 15, 2018

dwijnand commented Dec 15, 2018

Ichoran commented Dec 15, 2018

viktorklang commented Dec 15, 2018 via email

adriaanm commented Dec 15, 2018 via email

dwijnand commented Dec 15, 2018

dwijnand commented Dec 15, 2018

viktorklang commented Dec 15, 2018 via email • edited

dwijnand commented Dec 15, 2018

viktorklang commented Dec 15, 2018

adriaanm commented Dec 15, 2018 via email

dwijnand commented Dec 15, 2018

He-Pin commented Dec 16, 2018 • edited

NthPortal commented Dec 16, 2018

joshlemer commented Dec 16, 2018

viktorklang commented Dec 16, 2018 via email

He-Pin commented Dec 16, 2018

dwijnand commented Dec 16, 2018

som-snytt commented Dec 16, 2018 • edited

tarsa commented Dec 16, 2018

tarsa commented Dec 16, 2018

adriaanm commented Dec 16, 2018 via email

dwijnand commented Dec 16, 2018

OlegYch commented Dec 16, 2018

lrytz commented Dec 17, 2018

tarsa commented Dec 17, 2018

dwijnand commented Dec 17, 2018

Ichoran commented Dec 17, 2018

dwijnand commented Dec 17, 2018

NthPortal commented Dec 18, 2018 • edited

adriaanm commented Dec 20, 2018

dwijnand commented Dec 20, 2018

adriaanm commented Dec 20, 2018 via email

dwijnand commented Dec 21, 2018 • edited

dwijnand commented Dec 14, 2018 •

edited

Ichoran commented Dec 15, 2018 •

edited

viktorklang commented Dec 15, 2018 via email •

edited

He-Pin commented Dec 16, 2018 •

edited

som-snytt commented Dec 16, 2018 •

edited

NthPortal commented Dec 18, 2018 •

edited

dwijnand commented Dec 21, 2018 •

edited