Skip to content
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

Promise and Future could have a better toString #9488

Closed
scabug opened this issue Sep 25, 2015 · 7 comments
Closed

Promise and Future could have a better toString #9488

scabug opened this issue Sep 25, 2015 · 7 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Sep 25, 2015

I noticed that in 2.12.x, DefaultPromise has inherited the toString of the its new parent, AtomicReference.

scala> Promise.apply[String]()
res16: scala.concurrent.Promise[String] = List()

This unnecessarily leaks implementation details.

I think "" would be better. We might want to include the completion status, too.

Future could take the same treatment.

@scabug
Copy link
Author

scabug commented Sep 25, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9488?orig=1
Reporter: @retronym
Affected Versions: 2.11.8

@scabug
Copy link
Author

scabug commented Sep 25, 2015

@retronym said:
[~viktorklang] WDYT?

@scabug
Copy link
Author

scabug commented Sep 25, 2015

Viktor Klang (viktorklang) said:
AFAIK this has already been "fixed" in 2.12.x: https://github.com/scala/scala/blob/2.12.x/src/library/scala/concurrent/impl/Promise.scala#L46

However, I think the solution in 2.12.x is a bit dodgy in the sense that toString now has a temporal aspect to it, but perhaps that is not a problem since the same thing affects virtually every mutable object since the dawn of time.

@scabug
Copy link
Author

scabug commented Mar 11, 2016

@SethTisue said:
This was reported against 2.11.8 by @nilskp at https://groups.google.com/d/msg/scala-user/FjhQGH7TII0/DPz8tOVUAgAJ

I suggest we backport the 2.12 fix for 2.11.9.

@scabug
Copy link
Author

scabug commented Mar 11, 2016

@SethTisue said:
another possibility would be to restore the 2.11.7 behavior:

scala> concurrent.Promise.apply[String]()
res0: scala.concurrent.Promise[String] = scala.concurrent.impl.Promise$DefaultPromise@71dac704

@scabug
Copy link
Author

scabug commented Mar 11, 2016

Viktor Klang (viktorklang) said:
The appropriate fix is to move the toString overrides in 2.12 to 2.11

The old version is problematic, not only does it expose that a Future is also a Promise, but it also involved System.identityHashCode, which is not only problematic from the fact that it needs to be stored in the object header, but also because it conveys 0 meaning. Thoughts?

@scabug
Copy link
Author

scabug commented Mar 23, 2016

@lrytz said:
scala/scala#5056

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants