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

value classes crippled by nesting restriction #7685

Open
scabug opened this issue Jul 22, 2013 · 19 comments
Open

value classes crippled by nesting restriction #7685

scabug opened this issue Jul 22, 2013 · 19 comments

Comments

@scabug
Copy link

scabug commented Jul 22, 2013

Noting for posterity, because I tried for this during the development of value classes, and I don't doubt it's still wontfix:

Value classes are much less useful than they could have been. The prohibition on wrapping means that you can take exactly one step down any given value class path and are frozen there forever after. If you try to use value classes in a meaningful way you will run into this restriction constantly. A nontrivial design which is not hampered is not possible.

AFAIK the only reason for the restriction is the requirement that everything be reconstructable at runtime via pattern matching. It is kind of tragic to so severely limit the power of value classes to support typeless-language style programming. If you need to pattern match your way back out of nested value classes, you are Doing It Wrong.

Here is a typical example of where it hits, even before a second API-facing value class appears. Your value class can have non-value extension methods; your non-value class can have value class extension methods; but if you would like your value class to have value class extension methods (and when exactly would you NOT want this for your value class? "Never" is when) you cannot.

object a {
  final class Meter(val amount: Int) extends AnyVal {
    def add(x: Meter) = new Meter(amount + x.amount)
  }
  final implicit class MeterOps(val meter: Meter) extends AnyRef {
    def bippy(x: Meter) = new Meter(meter.amount * x.amount)
  }
  def f(x: Meter) = (x add x) -> (x bippy x)
}

object b {
  final class Meter(val amount: Int) extends AnyRef {
    def add(x: Meter) = new Meter(amount + x.amount)
  }
  final implicit class MeterOps(val meter: Meter) extends AnyVal {
    def bippy(x: Meter) = new Meter(meter.amount * x.amount)
  }
  def f(x: Meter) = (x add x) -> (x bippy x)
}

object c {
  final class Meter(val amount: Int) extends AnyVal {
    def add(x: Meter) = new Meter(amount + x.amount)
  }
  final implicit class MeterOps(val meter: Meter) extends AnyVal {
    def bippy(x: Meter) = new Meter(meter.amount * x.amount)
  }
  // ./a.scala:25: error: value class may not wrap another user-defined value class
  //   final implicit class MeterOps(val meter: Meter) extends AnyVal {
  //                                     ^
  // one error found
  def f(x: Meter) = (x add x) -> (x bippy x)
}
@scabug
Copy link
Author

scabug commented Jul 22, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7685?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:

AFAIK the only reason for the restriction is the requirement that everything be reconstructable at runtime via pattern matching. It is kind of tragic to so severely limit the power of value classes to support typeless-language style programming. If you need to pattern match your way back out of nested value classes, you are Doing It Wrong.

Or in other words, nested value class would need to have outer pointer to preserve semantics of path-dependent types at runtime but then value class would erase to two fields and not one which is not supported at the moment.

If we are complaining about value class restrictions, how about:

class Foo[T](val x: T) extends AnyVal

class Test[T] {
	def test: Foo[T] => Foo[T] = foo => foo
}

If you try to compile it you get:

Test.scala:4: error: bridge generated for member method apply: (foo: Foo[T])Foo[T] in anonymous class $anonfun
which overrides method apply: (v1: T1)R in trait Function1
clashes with definition of the member itself;
both have erased type (v1: Object)Object
	def test: Foo[T] => Foo[T] = foo => foo
                                         ^
one error found

Which is super confusing error that no regular Scala user can possibly comprehend. However, what Scala compiler hacker can read from it is that if you have a generic value class that erases to object then there's no way you can create a lambda that takes or returns such a class. One could ask why do we even allow generic value classes if a core feature of Scala (lambdas) doesn't work with them and gives a super cryptic error message.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said:
In principle, "clashes with definition of the member itself" is a bug, not a feature of the design.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:
Well, the bug is to allow generic value classes. There's no way we can fix bridge method problem because the clash is simply unavoidable within our current scheme of erasing value classes.

One could imagine that value classes where part of specialization so the specialized (containing erased value class in signature) method would be called apply$sp... instead of just apply. This way we would avoid name clash but it would mean we are doing complete rewrite of value classes.

I believe that in current form, value classes that erase to object should be disallowed.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said (edited on Jul 22, 2013 9:09:25 PM UTC):
The clash may be unavoidable, but it doesn't follow that it can't be fixed. The conflicting bridge method is being created to forward to the method with which it is conflicting. What is the problem with just not generating the bridge, if the implementation has the same signature?

Here, this has the same problem without value class involvement. The bridge method, were it to be generated, will conflict with the definition of the member itself.

class Bippy {
  val test: Object => Object = x => x
  // Only gets (Object)Object
}

But it isn't a problem, because we use the implementation to satisfy the requirement for an (Object)Object method. In all other cases, there is a bridge:

class Dingo {
  val test: String => String = x => x
  // Gets (String)String and (Object)Object
}

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:
It is actually different situation when value classes are involved. Let me demonstrate the problem:

class Foo[T](val x: T) extends AnyVal

class FooFunction extends Function1[Foo[Int], Foo[Int]] {
  def apply(foo: Foo[Int]): Foo[Int] = new Foo(foo.x+1)
}

val f: FooFunction = new FooFunction
val foo = new Foo(12) // foo has erased type Int
f.apply(foo) // no boxing here, we should go straight to apply(I)I method
(f: Function1[Foo[Int], Foo[Int]]).apply(foo) // we need to box into Foo before calling apply(LObject)LObject and then that method unboxes and forwards to apply(I)I

In this case everything works ok. However, replace Int by Object and you'll see we have a clash, you need two apply's one which assumes you have Foo passed in (and unwraps it) and the other one which assumes you have underlaying content of the method passed in. They have different method bodies but exactly the same signature. That's the problem.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said (edited on Jul 22, 2013 9:32:01 PM UTC):
So multiplex the (Object)Object method. I guess it goes bad if people are throwing around Foo[Foo[Int]]s, but in what must be the common case:

def apply(x: Object): Object = if (x instanceof Foo) apply(x.x) else actualImpl(x)

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said:
...whereupon the connection between the bridge problem and the problem I was reporting becomes more apparent.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:
Yeah, multiplexing is an option but it only gets you that far.

My point is that I'm very skeptical that we can make using generic value classes a smooth experience without any gotchas like we are discussing here. If we cannot make it really good then I'd remove it instead of making people to scratch their heads and hunt bug database for tickets discussing cryptic concepts like bridge methods.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:
Ah yes, sorry for hijacking this ticket!

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said:
I wouldn't say you hijacked it, because the question of "how many levels of unwrapping does this T/Object require" and "what is the original type of this T/Any" are similar questions if value classes can be nested arbitrarily; so the bridge issue illustrates that we would very likely fail even if we relaxed the explicit reconstruction requirement.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@gkossakowski said:
Ah, good point. I didn't realize that. Fundamentally, it looks like with value classes we are trying to bend erasure rules too far to fit into box (pun intended) JVM gives us. I believe much better approach is to deal with box-unboxing optimization at JVM level (by nudging JIT). It seems like an excellent topic for JVM Language summit next week.

@scabug
Copy link
Author

scabug commented Jul 22, 2013

@paulp said:
It's no coincidence I opened this ticket just now, I'm working on my talk for said summit.

@scabug
Copy link
Author

scabug commented Oct 28, 2013

@paulp said (edited on Oct 28, 2013 4:39:16 PM UTC):
Regardless of the insolubility of the problem in the general case, it would sure be nice to improve on this. Bytecode transcripts are after compiling under -optimise with current head in the neighborhood of 2.11.0-M6.

final class Foo(val value: Int) extends AnyVal

final object Foo {
  @inline def apply(x: Int): Foo = new Foo(x)

  // Isn't allowed to extend AnyVal due to wrapping value class
  // Doesn't do any good not to use an implicit class
  @inline implicit final class FooOps(val x1: Foo) {
    @inline def bar(x2: Foo): Foo = Foo(x1.value + x2.value)
  }
}

class A {
  // Super-optimal - implies possible omniscience on part of constant folder
  def superoptimal: Int = 5 + 5
  // 0: bipush        10
  // 2: ireturn

  // Optimal - implies only that inlining works
  def optimal: Int = (5: Int) + (5: Int)
  // 0: iconst_5
  // 1: iconst_5
  // 2: iadd
  // 3: ireturn

  def actual: Int = (Foo(5) bar Foo(5)).value
  //  0: getstatic     #20                 // Field Foo$.MODULE$:LFoo$;
  //  3: getstatic     #20                 // Field Foo$.MODULE$:LFoo$;
  //  6: astore_1
  //  7: astore_2
  //  8: new           #22                 // class Foo$FooOps
  // 11: dup
  // 12: iconst_5
  // 13: invokespecial #26                 // Method Foo$FooOps."<init>":(I)V
  // 16: getstatic     #20                 // Field Foo$.MODULE$:LFoo$;
  // 19: astore_3
  // 20: astore        4
  // 22: getstatic     #20                 // Field Foo$.MODULE$:LFoo$;
  // 25: aload         4
  // 27: invokevirtual #29                 // Method Foo$FooOps.x1:()I
  // 30: iconst_5
  // 31: iadd
  // 32: istore        6
  // 34: astore        5
  // 36: iload         6
  // 38: ireturn
}

@scabug
Copy link
Author

scabug commented Apr 3, 2015

Guillaume Martres (Smarter) said (edited on Apr 3, 2015 11:36:58 PM UTC):
So, I'm currently implementing value classes in Dotty (see scala/scala3#411) and I was trying to understand why nested value classes where forbidden in scalac, but I have to admit that I'm not sure I understand what Paul means when he says:
bq. AFAIK the only reason for the restriction is the requirement that everything be reconstructable at runtime via pattern matching.
My best guess is that this is related to something like:

case class Foo(val unFoo: Int) extends AnyVal

case class Bar(val unBar: Foo) extends AnyVal

object O {
  def thing(x: Any) = {
    x match {
      case Bar(Foo(i)) => i
      case _ => 0
    }
  }
  def test() = {
    val bf = new Bar(new Foo(1))
    thing(bf)
  }
}

But this work just fine in my pull request, and it doesn't seem impossible to handle in scalac either. Can someone give me an example of something that is likely to fail with nested value classes? Thanks.

@scabug
Copy link
Author

scabug commented Jun 20, 2016

@lrytz said:
WIP branch here: https://github.com/scala/scala/compare/2.12.x...lrytz:t7685?expand=1

I probably won't have time to work more on this right now.

@scabug
Copy link
Author

scabug commented Aug 10, 2016

@SethTisue said:
Lukas: Assigned to 2.13 on the optimistic assumption you might want to return to this, but feel free to reassign to Backlog.

@scabug
Copy link
Author

scabug commented Jan 5, 2017

@dwijnand said:
I hope to be able to solve this. Hopefully within 2.12.x.

@szeiger szeiger removed this from the 2.13.0-M2 milestone Jul 20, 2017
@adriaanm adriaanm modified the milestones: 2.13.0-M3, 2.13.0-M4 Jan 30, 2018
@SethTisue
Copy link
Member

anyone working on this...?

@lrytz lrytz removed this from the 2.13.0-M4 milestone Apr 23, 2018
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

6 participants