Some tests on jdk10 #6889
Some tests on jdk10 #6889
Conversation
|
@lrytz Maybe you have a quick answer why this check fails on Java 10. (safeToInline is expected false.) |
|
partest is tweaked to handle filtering check file on No attempt to do java modules: avoid I tested on OpenJDK Java 10 not 9. There are still errors in other categories under |
|
Also taking the opportunity to remove extraneous flags files. It would be nice if |
It seems the annotation class moved in 10 checks for |
We need to tweak the desugaring of structural calls a little bit to avoid this. Instead of: Which is called from, e.g. scala> class Reflect { def foo = ""; def bar = { this.asInstanceOf[{ def foo: String }].foo }}
defined class ReflectWhich compiles to: When running on JDK9+, we should call First, we'll need to put some JDK-dependent code in Then, we need to change the compilation of structural calls to pass the receiver, rather than reciever class down. For source compatibility, we should probably leave the current signature of |
|
I think that test/files/run/t1167.scala is failing because we're emitting invalid metadata in the |
|
The comment in the test is right, the javadoc says |
|
@lrytz Thanks! I'd noticed the package change elsewhere, but too lazy to drill down here. |
| def tr[T](m: => T): String = try { | ||
| val r = m | ||
| if (r == null) "null" | ||
| else r.toString | ||
| } catch { case e: InternalError => e.getMessage } | ||
|
|
||
| def assertNotAnonymous(c: Class[_]) = { | ||
| val an = try { | ||
| def assertNotAnonymous(c: Class[_]) = assert8(!isAnonymous(c), s"$c is anonymous") |
There was a problem hiding this comment.
This check also flipped.
There was a problem hiding this comment.
IIUC, this is just documenting the evolving status quo for java reflect interaction.
|
Dread |
|
From the migration guide:
That probably foils: scala/test/scalacheck/scala/tools/nsc/scaladoc/SettingsUtil.scala Lines 10 to 21 in 71f9b43 |
|
Something like this ought to be cross JDK compatible: if (Thread.currentThread.getContextClassLoader == ClassLoader.getSystemClassLoader)
settings.usejavacp.value = true
else
settings.embeddedDefaults(ourClassLoader) // assume we're under SBT |
|
Thanks @retronym I dusted off |
|
@retronym OK to squash to one commit? Not sure any history worth preserving. I was about to look at osgi test breakage, but maybe one can merge before fixing all the things. |
|
Could you please keep the flag file removal separate? Otherwise squashing
sounds fine.
|
|
Worth mentioning that zulu 10 crashed twice this week trying to sbt. |
|
What's left? OSGi reflection tests, I presume? Once |
|
Yes, I keep running out of time windows. I'll pursue a couple of clean-ups that I forgot while squashing... Edit: the weekend is coming up. Maybe book this PR and I'll follow up. |
| @@ -61,3 +62,67 @@ public void Test4$Foo12.name_$eq(java.lang.String) | |||
| 99 | |||
| dylan | |||
| 2 | |||
| #partest !java8 | |||
There was a problem hiding this comment.
It wouldn't be brain surgery for partest --update-check to support existing filters, or possibly under a flag --check-jvm.
| /** Assert on Java 8, but on later versions, just print if assert would fail. */ | ||
| def assert8(b: => Boolean, msg: => Any) = | ||
| if (ScalaVersion(javaSpecVersion) == version8) assert(b, msg) | ||
| else if (!b) println(s"assert not $msg") |
There was a problem hiding this comment.
Is this still useful or does it hide migration errors?
There was a problem hiding this comment.
Not used, actually.
|
First, fix the bane of my existence |
| assert(callee.safeToInline == safeToInline) | ||
| ======= | ||
| assertEquals("safeToInline", safeToInline, callee.safeToInline) | ||
| >>>>>>> 283e089... Fix caller sensitive test per lrytz |
Partest accepts version filter for arbitrary `javaN` or `java9+` or `!java8`. Generally, avoid deprecated API. Strip module name from output. scalap classpath test for dot has only dot. The anonymous class is now really incognito. Ignore module reflection WARNINGs for now. Fix path setting for scalacheck/scaladoc The previous code attempted to reconstruct the class path from the class loader, but the class loader implementations change in Java 9. The more direct approach is -usejavacp if no class loader has been created below the app CL, and to use the test CL as a parent otherwise.
|
Then fix the test munged by a git conflict. |
Flags are extraneous for language features. For single-source tests, flags can be inserted at the top of the file in the form `// scalac: -Xlint`.
|
Consider the progress booked! |
|
thank you so much, Som, for this epic PR!! |
| def hasCallerSensitiveAnnotation(methodNode: MethodNode): Boolean = | ||
| methodNode.visibleAnnotations != null && | ||
| methodNode.visibleAnnotations.stream.filter(ann => | ||
| ann.desc == "Lsun/reflect/CallerSensitive;" || ann.desc == "Ljdk/internal/reflect/CallerSensitive;" |
There was a problem hiding this comment.
Would this fix the issue I described in the comment below?
If so, it would be nice to backport it to Scala 2.12.
There was a problem hiding this comment.
Very clever to notice. But I thought only Spark was still stuck on 2.12. I'll try to make time to sort that out, next a modern version of Java is on my class path.
There was a problem hiding this comment.
Libraries typically support multiple Scala versions. Kafka supports Scala 2.11 and 2.12 today and it will support 2.13 soonish.
There was a problem hiding this comment.
Sorry, that was my Spark joke for the day. Some people use sbt using 2.10. But thanks again, it's fun to have an opportunity to explore the Java 9/10/11 event horizon.
Fixes scala/scala-dev#522