scala / community-build

Scala 2 community build — a corpus of open-source repos built against Scala nightlies
Apache License 2.0
139 stars 59 forks source link

2.13: new Scala SHA #1677

Closed SethTisue closed 1 year ago

SethTisue commented 1 year ago

~https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4574/~ ~https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4576/~ ~https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4577/~ https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4599/

SethTisue commented 1 year ago

singleton-ops

[singleton-ops] [error] /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.20/project-builds/singleton-ops-43ff529071c9b3e9a0869a02f971698722b8df28/src/test/scala/singleton/twoface/TwoFaceStringSpec.scala:14:27: type mismatch;
[singleton-ops] [error]  found   : ValueOf[String]
[singleton-ops] [error]  required: ValueOf[String("Something")]
[singleton-ops] [error] Note: String >: String("Something"), but class ValueOf is invariant in type T.
[singleton-ops] [error] You may wish to investigate a wildcard type such as `_ >: String("Something")`. (SLS 3.2.10)
[singleton-ops] [error]     val a = TwoFace.String[W.`"Something"`.T]
[singleton-ops] [error]                           ^

shapeless

[shapeless] [error] /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.20/project-builds/shapeless-1448284f88e35afd260e5bd6d419e528e292758a/core/shared/src/test/scala/shapeless/product.scala:320:19: could not find implicit value for parameter toMap: shapeless.ops.product.ToMap.Aux[ProductTests.this.Foo,K,V]
[shapeless] [error]       val m = foo.toMap
[shapeless] [error]                   ^

scapegoat

[scapegoat] [error] Failed tests:
[scapegoat] [error]     com.sksamuel.scapegoat.inspections.unnecessary.UnnecessaryConversionTest
SethTisue commented 1 year ago

Scala merged PRs in this time span:

* 7db02feacd - (HEAD -> 2.13.x, origin/HEAD, origin/2.13.x) Merge pull request #10425 from RustedBones/equals-converters (3 days ago) <Lukas Rytz>
* 0f5746fd7c - Merge pull request #10352 from som-snytt/issue/12757-glblub-loop (3 days ago) <Lukas Rytz>
* 0842f23f60 - Merge pull request #10424 from som-snytt/issue/12800-enum-exhaustivity (3 days ago) <Lukas Rytz>
* 6b16203c3d - Merge pull request #10414 from som-snytt/issue/12792-complement-constant (4 days ago) <Lukas Rytz>
* ebcee622d3 - Merge pull request #10404 from som-snytt/issue/12787-inliner (9 days ago) <som-snytt>
* 6e6bf056bb - Merge pull request #10434 from SethTisue/merge-2.12-to-2.13-20230612 (9 days ago) <Seth Tisue>
* 6243e90292 - Merge pull request #10426 from lrytz/sbt1.9 (2 weeks ago) <Lukas Rytz>
* 952c36e638 - Merge pull request #10411 from som-snytt/issue/12770-string-exhaust (3 weeks ago) <Dale Wijnand>
* a2adfb3bee - Merge pull request #10413 from som-snytt/tweak/print-syms (3 weeks ago) <Seth Tisue>
* 94c8030c94 - Merge pull request #10368 from som-snytt/issue/12769-no-sources (3 weeks ago) <Seth Tisue>
SethTisue commented 1 year ago

@lrytz @som-snytt I'll take a closer look and maybe do some bisecting... but y'all feel free to look as well :-)

SethTisue commented 1 year ago

I did some bisecting and it seems that shapeless and singleton-ops regressed in scala/scala#10352 and scapegoat regressed in some earlier PR. I'll do some more bisecting and investigating when next at keyboard.

som-snytt commented 1 year ago

Sorry, I missed this call to arms. I guess by scapegoat, you don't mean me, but a project.

SethTisue commented 1 year ago

for Scapegoat, the relevant PR is https://github.com/scala/scala/pull/10414 ("Fold selection of unary ops")

SethTisue commented 1 year ago

for Scapegoat, our fork was out of date, but the problem remains reproducible even after refreshing the fork. it's also reproducible outside of dbuild, but you have to publish scalameta 4.7.8 locally for the Scala version you're testing

the test in question is UnnecessaryConversionTest's "when invoking toInt on an integer literal":

      "when invoking toInt on an integer literal" in {
        val code =
          """object Example extends App {
            |  val a = 3.toInt        // NullPointerException here (v1.3.6).
            |  val b = (10 / 5).toInt // NullPointerException here (v1.3.6).
            |}""".stripMargin

        compileCodeSnippet(code)
        compiler.scapegoat.feedback.warnings.size shouldBe 2
      }

and the failure is

[info]   - when invoking toInt on an integer literal *** FAILED *** (48 milliseconds)
[info]     0 was not equal to 2 (UnnecessaryConversionTest.scala:42)

where the missing warnings are "Unnecessary toInt on instance of Int"

so, I don't think this is a regression in Scala. it's good that we now constant-fold this. as far as I can see, Scapegoat should adjust. perhaps the compiler plugin ought to run before constant folding?

I will exclude the test for now. cc @sksamuel @lolgab @saeltz

SethTisue commented 1 year ago

for singleton-ops, the failing test is

  property("Safe Creation[]") = {
    val a = TwoFace.String[W.`"Something"`.T]
    a.getValue == "Something" && a.isLiteral
  }

it is easily reproducible outside of dbuild (with ++2.13.12-bin-7db02fe!)

note that Shapeless is involved (W is Shapeless's Witness, with type member T)

not obvious to me what's going on. cc @fthomas @milessabin

SethTisue commented 1 year ago

for Shapeless, the problem reproduces outside dbuild, but you'll want the changes in https://github.com/milessabin/shapeless/pull/1315, not yet merged (the repo has fatal warnings enabled and 2.13.11 caused new warnings to appear)

there are a lot of "implicit not found" errors, it isn't just the sample error I showed above. the full errors are at https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4577/artifact/logs/shapeless-build.log

I have no hypothesis about what's going on here

milessabin commented 1 year ago

I'm afraid I don't have time to work on this. Maybe ask @joroKr21?

Eyeballing scala/scala#10352, the changes in GlbLubs.scala and TypeConstraints.scala look like they might have introduced some unexpected semantic changes along with the intended stack-safety and performance improvements. I could easily imagine that subtly changing type inference and hence implicit resolution.

shapeless is likely to be very sensitive to this, but I wouldn't be surprised if there's more breakage in the wild.

joroKr21 commented 1 year ago

I can take a look next week, I'm out for the weekend

som-snytt commented 1 year ago

I guess I'm in for the weekend.

som-snytt commented 1 year ago

Joyfully, it seems ScalaTest does not support scalaHome setting. Whenever I have to try something that depends on ScalaTest, I find myself checking twitter more often. Please disabuse me as necessary.

[error] Caused by: java.lang.ClassNotFoundException: scala.Product
[error]         at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
[error]         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:588)
[error]         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
[error]         at java.base/java.lang.ClassLoader.defineClass1(Native Method)
[error]         at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1013)
[error]         at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
[error]         at java.base/java.net.URLClassLoader.defineClass(URLClassLoader.java:524)
[error]         at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:427)
[error]         at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:421)
[error]         at java.base/java.security.AccessController.doPrivileged(AccessController.java:714)
[error]         at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:420)
[error]         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:588)
[error]         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:575)
[error]         at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
[error]         at java.base/java.lang.Class.forName0(Native Method)
[error]         at java.base/java.lang.Class.forName(Class.java:496)
[error]         at java.base/java.lang.Class.forName(Class.java:475)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.loadSuiteClass(Framework.scala:408)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.suiteClass$lzycompute(Framework.scala:416)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.suiteClass(Framework.scala:416)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.accessible$lzycompute(Framework.scala:417)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.accessible(Framework.scala:417)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.shouldDiscover$lzycompute(Framework.scala:420)
[error]         at org.scalatest.tools.Framework$ScalaTestTask.shouldDiscover(Framework.scala:419)
[error]         at org.scalatest.tools.Framework$ScalaTestRunner.$anonfun$tasks$2(Framework.scala:752)
[error]         at org.scalatest.tools.Framework$ScalaTestRunner.$anonfun$tasks$2$adapted(Framework.scala:750)
[error]         at scala.collection.ArrayOps$WithFilter.map(ArrayOps.scala:90)
[error]         at org.scalatest.tools.Framework$ScalaTestRunner.tasks(Framework.scala:750)
[error]         at sbt.TestRunner.tasks(TestFramework.scala:119)
som-snytt commented 1 year ago

For the shapeless error, https://github.com/scala/scala/pull/10452

Contrary to Miles' conjecture, it was not subtle. However, I haven't investigated yet how it breaks.

After creating a minimal test on the shapeless side, I realized reversion was quicker than analysis. But I am motivated to understand it at some future day of leisure.

joroKr21 commented 1 year ago

Thanks 👍 - in this case it's the reference equality on types which comes up every now and then as the source of bugs.

Screenshot 2023-07-03 at 00 15 12
saeltz commented 1 year ago

so, I don't think this is a regression in Scala. it's good that we now constant-fold this. as far as I can see, Scapegoat should adjust. perhaps the compiler plugin ought to run before constant folding?

@SethTisue, thanks for bringing this up! Could you remind me again in which phase the constant-folding is happening? Scapegoat currently runs after typer and before patmat. I thought it was a sub-phase of type-checking.

The other way to solve this would be for Scapegoat to only run this inspection for Scala 2.12.

SethTisue commented 1 year ago

@saeltz let's discuss on a scapegoat ticket instead of here

saeltz commented 1 year ago

let's discuss on a scapegoat ticket instead of here

@SethTisue, I created https://github.com/scapegoat-scala/scapegoat/issues/754.

SethTisue commented 1 year ago

verified that Som's followup PR fixed shapeless and singleton-ops

there is now a scalafmt failure, but I'm going to merge this anyway and deal with that separately (issue: #1680)