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

fastOptJS error in scalaz 7.3 with Scala.js 1.0.0 #3953

Open
xuwei-k opened this issue Feb 8, 2020 · 9 comments
Open

fastOptJS error in scalaz 7.3 with Scala.js 1.0.0 #3953

xuwei-k opened this issue Feb 8, 2020 · 9 comments
Labels

Comments

@xuwei-k
Copy link
Contributor

@xuwei-k xuwei-k commented Feb 8, 2020

[error] https://raw.githubusercontent.com/scalaz/scalaz/382bb66e804ef0ec56ff4a4eb3508faebb916d36/tests/src/test/scala/scalaz/StreamTTest.scala(160:33:MethodDef): The signature of scalaz.StreamTTest$$anonfun$$nestedInanonfun$new$111$1.applyOrElse(scalaz.StreamT$Step,scala.Function1)java.lang.Object, which is (List(AnyType, ClassType(ClassName<scala.Function1>)),AnyType), does not match its name (should be (List(ClassType(ClassName<scalaz.StreamT$Step>), ClassType(ClassName<scala.Function1>)),AnyType)).
[error] There were 1 IR checking errors.
[error] (testsJS / Test / fastOptJS) There were 1 IR checking errors.
@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Feb 9, 2020

Thanks for the report. I will look into it once we're through with all the release steps.

Since the IR error is in the tests, it probably means that the main artifacts, to be published, are correct. You could comment out the problematic test, and if things pass, then you can safely publish the main artifacts, I believe.

@sjrd sjrd added the bug label Feb 9, 2020
@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Feb 9, 2020

It would help us fix the issue if you manage to provide a minimization.

@xuwei-k

This comment has been minimized.

Copy link
Contributor Author

@xuwei-k xuwei-k commented Feb 10, 2020

package example

sealed class StreamT[M[_], A](val step: M[StreamT.Step[A, StreamT[M, A]]])

object StreamT {
  sealed abstract class Step[A, S] extends Product with Serializable

  case class Done[A, S]() extends Step[A, S]
}

object Main {
  import StreamT._

  implicit class AnyOps[A](actual: => A) {
    def mustMatch(f: PartialFunction[A, Boolean]): Unit = {
      val act = actual
      def test = f.isDefinedAt(act) && f(act)
      def koMessage = "%s does not satisfy partial function".format(act)
      if (!test)
        sys.error(koMessage)
    }
  }

  type Id[A] = A

  def x[A](s: StreamT[Id, A]) = s.step mustMatch {
    case Done() => true
  }

  def main(args: Array[String]): Unit = {
    println(x(null))
  }
}
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Feb 10, 2020

Further reduction:

package example

import scala.language.higherKinds

sealed class StreamT[M[_]](val step: M[Step[StreamT[M]]])

sealed abstract class Step[S]

object Main {
  implicit class AnyOps[A](actual: A) {
    def mustMatch(f: PartialFunction[A, Boolean]): Unit = ()
  }

  type Id[A] = A

  def main(args: Array[String]): Unit = {
    (??? : StreamT[Id]).step mustMatch {
      case _ => true
    }
  }
}

Notably: The implicit conversion and the type function seem to be required to reproduce.

@sjrd sjrd changed the title fastOptJS error in scalaz 7.3 with scala-js 1.0.0 fastOptJS error in scalaz 7.3 with Scala.js 1.0.0 Feb 11, 2020
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Feb 11, 2020

So it seems the problem is that we take the param symbols from different locations:

This is (for now), the only difference I can see between the ways we retrieve the types :-/

Both genParam and paramOrResultTypeRef seem to be calling convert at the end.

@errikos

This comment has been minimized.

Copy link

@errikos errikos commented Feb 11, 2020

@gzm0 you are right.

However, this seems to be an issue in scalac itself. I opened a bug report for it: scala/bug#11884.

errikos added a commit to errikos/scala-js that referenced this issue Feb 12, 2020
… aliases

The issue occurs due to a bug in scalac, where Defdef vparamss and the
respective symbol have inconsistent types. For more information, please
see: scala/bug#11884
errikos added a commit to errikos/scala-js that referenced this issue Feb 12, 2020
The issue occurs due to a bug in scalac, where Defdef vparamss and the
respective symbol have inconsistent types. For more information, please
see: scala/bug#11884
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Feb 12, 2020

Just to confirm my understanding (w.r.t. scala/bug#11884 (comment)): The reason we need all this uglyness and patching (and cannot simply use tpe.params) is because the refs to the parameters have the wrong types?

@errikos

This comment has been minimized.

Copy link

@errikos errikos commented Feb 12, 2020

Just to confirm my understanding (w.r.t. scala/bug#11884 (comment)): The reason we need all this uglyness and patching (and cannot simply use tpe.params) is because the refs to the parameters have the wrong types?

I don't know about "wrong", but I would say that "inconsistent" is a more accurate word. The Defdef's symbol has the correct types (which the patch uses), but Defdef.vparamss has the wrong ones.

Plus, the easiest way to do this is to patch the IR Tree after it has been generated, in order to avoid touching the generation process itself, which would probably introduce far more code changes.

@sjrd

This comment has been minimized.

Copy link
Member

@sjrd sjrd commented Feb 21, 2020

PR in scala/scala: scala/scala#8720

@sjrd sjrd added the upstream label Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.