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

Linking error after the optimizer for someInt.toDouble.compareTo(double) #3085

Closed
sjrd opened this issue Aug 7, 2017 · 0 comments · Fixed by #3086
Closed

Linking error after the optimizer for someInt.toDouble.compareTo(double) #3085

sjrd opened this issue Aug 7, 2017 · 0 comments · Fixed by #3086
Assignees
Labels
bug
Milestone

Comments

@sjrd
Copy link
Member

@sjrd sjrd commented Aug 7, 2017

The following snippet:

object Main {
  def main(args: Array[String]): Unit =
    foo(5)

  def foo(x: Int): Unit =
    bar(x.toDouble)

  @inline def bar(x: Double): Unit =
    println(x.compareTo(5.5))
}

causes the following linking error after the optimizer has run:

[error] Referring to non-existent method java.lang.Integer.compareTo(java.lang.Double)scala.Int
[error]   called from helloworld.HelloWorld$.main([java.lang.String)scala.Unit
[error]   called from core module module initializers
[error] involving instantiated classes:
[error]   helloworld.HelloWorld$

This is because the type of x in bar() is refined to be int because of the inlining. And then we search for compareTo__D__B in j.l.Integer instead of j.l.Double, which causes the breakage.

This issue is similar to #2184 and #2780, and to the reason we define compareTo(Byte/Short) in j.l.Integer (here). However, the present issue shows that it is more severe than I previously thought. It is not limited to Byte and Short being treated as Ints because there are no bytes and shorts in the IR. In general, it is also problematic because int <: double (and float <: double) in the IR, which allows the optimizer to refine a double into an int.

This means that this issue would not be fixed simply by adding bytes and shorts in the IR. We need those ugly bridges in Integer (and Float) anyway.

Worse, there is a more annoying issue. The following snippet:

object Main {
  def main(args: Array[String]): Unit =
    foo(5)

  def foo(x: Int): Unit =
    bar(x.toDouble)

  @inline def bar(x: Double): Unit =
    foobar(x)

  @inline def foobar(x: Comparable[java.lang.Double]): Unit =
    println(x.compareTo(5.5))
}

links, but fails at run-time with a CCE:

[error] scala.scalajs.runtime.UndefinedBehaviorError: An undefined behavior was detected: 5.5 is not an instance of java.lang.Integer
[error]     at $c_sjsr_UndefinedBehaviorError.$c_jl_Throwable.fillInStackTrace__jl_Throwable (StackTrace.scala:52:51)
[error]     at $c_sjsr_UndefinedBehaviorError.fillInStackTrace__jl_Throwable (UndefinedBehaviorError.scala:24:34)
[error]     at $c_sjsr_UndefinedBehaviorError.$c_jl_Throwable.init___T__jl_Throwable (Throwables.scala:12:19)
[error]     at $c_sjsr_UndefinedBehaviorError.init___T__jl_Throwable (Throwables.scala:156:46)
[error]     at $c_sjsr_UndefinedBehaviorError.init___jl_Throwable (UndefinedBehaviorError.scala:20:5)
[error]     at $throwClassCastException (scalajsenv.js:172:1)
[error]     at $asInt (scalajsenv.js:619:1)
[error]     at $c_Lhelloworld_HelloWorld$.main__AT__V (Integer.scala:8:13)
[error]     at Object.<anonymous> (helloworld-fastopt.js:2912:30)
[error]     at Object.<anonymous> (helloworld-fastopt.js:2913:4)

Here, the problem is that we end up calling the bridge j.l.Integer.compareTo(Object) instead of j.l.Double.compareTo(Object). But the former does not accept a Double as its parameter; it only accepts Ints.

That problem cannot be fixed by adding the relevant method in j.l.Integer, because it already exists, but does the wrong thing.

I'm afraid this issue is unfixable while the IR specifies that int <: double and int <: Integer/double <: Double. Since int <: Integer is critical to JS interop, I think we have to abandon int <: double in the IR. Instead, explicit conversions of the form UnaryOp(UnaryOp.IntToDouble, x) will have to be inserted, even though FunctionEmitter would erase them at the end of the day.

@sjrd sjrd added the bug label Aug 7, 2017
@sjrd sjrd self-assigned this Aug 7, 2017
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 7, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 7, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 7, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 7, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 8, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 9, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 9, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 12, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 12, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
sjrd added a commit to sjrd/scala-js that referenced this issue Aug 13, 2017
* Add types for `char`, `byte` and `short` in the IR.
* Remove the subtyping relationships between primitive types, for
  example `int <: double`.

Instead of the subtyping relationships, we introduce explicit
widening conversions as `UnaryOp`s. In order to avoid widening
`int`s to `double`s for the purpose of comparing them, we separate
`Num_<` and friends into `Int_<` and `Double_<`. For floats, we do
introduce the conversions in that case.

There is one subtlety for `char` (`CharType`): it is *not* a
subtype of `any`. That's because `char`s must always be boxed in
`j.l.Character` when they are assigned to an `any`.

This fixes scala-js#3085, and provides a cleaner solution to the old
issues scala-js#2184 and scala-js#2780.
@sjrd sjrd closed this in #3086 Aug 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.