Fix #8634: Support -release option #10746

prolativ · 2020-12-10T16:55:29Z

A port of https://github.com/scala/scala/pull/6362/files with some improvements
When running scalac on JDK 9+ the -release option assures that code is compiled with classes specific to the release available on the classpath.
This applies to classes from the JDK itself and from external jars.
If the compilation succeeds, bytecode for the specified release is produced.
-target option gets renamed to -Xtarget. Using -release instead is preferred since -Xtarget sets the bytecode version without any checks so this might lead to producing code that breaks at runime

smarter

Otherwise LGTM!

smarter · 2020-12-11T16:24:15Z

compiler/src/dotty/tools/io/ZipArchive.scala

@@ -112,9 +112,15 @@ abstract class ZipArchive(override val jpath: JPath) extends AbstractFile with E
  def close(): Unit
 }
 /** ''Note:  This library is considered experimental and should not be used unless you know what you are doing.'' */
-final class FileZipArchive(jpath: JPath) extends ZipArchive(jpath) {
+final class FileZipArchive(jpath: JPath, release: Option[String] = None) extends ZipArchive(jpath, release) {


I suggest removing the default param so we don' forget to pass release in some situations:

Suggested change

final class FileZipArchive(jpath: JPath, release: Option[String] = None) extends ZipArchive(jpath, release) {

final class FileZipArchive(jpath: JPath, release: Option[String]) extends ZipArchive(jpath, release) {

smarter · 2020-12-11T16:31:12Z

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

+
+  protected def supportedReleaseVersions: List[String] =
+    if scala.util.Properties.isJavaAtLeast("9") then
+      val jdkVersion = scala.util.Properties.javaSpecVersion.stripPrefix("1.").toInt


In DirectoryClassPath we use JDK9Reflectors.runtimeVersionMajor(JDK9Reflectors.runtimeVersion()).intValue(), I think it would be better to use that everywhere since that's the recommended way to get the version on Java 9+.

smarter · 2020-12-11T16:34:55Z

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -123,6 +136,7 @@ class ScalaSettings extends Settings.SettingGroup with CommonScalaSettings {
  val XignoreScala2Macros: Setting[Boolean] = BooleanSetting("-Xignore-scala2-macros", "Ignore errors when compiling code that calls Scala2 macros, these will fail at runtime.")
  val XimportSuggestionTimeout: Setting[Int] = IntSetting("-Ximport-suggestion-timeout", "Timeout (in ms) for searching for import suggestions when errors are reported.", 8000)
  val Xsemanticdb: Setting[Boolean] = BooleanSetting("-Xsemanticdb", "Store information in SemanticDB.").withAbbreviation("-Ysemanticdb")
+  val Xtarget: Setting[String] = ChoiceSetting("-Xtarget", "target", "Emit bytecode for the specified version of the Java platform. This setting is ignored if -release is specified.", supportedTargetVersions, supportedTargetVersions.head) withAbbreviation "--Xtarget"


This setting is ignored if -release is specified.

That's a bit confusing, I think it's fine to let -Xtarget override the target set with -release.

For javac you get an error if you try to use -source and -target at the same time. That would be the best option IMHO but I'm not sure if this can be easily achieved with our current approach to parsing command line options.
What would be the purpose of using -source and -Xtarget at the same time with different values? Then running scalac on JDK X you would try to compile code with classes from JDK Y on classpath to emit bytecode to be run on JDK Z, which would be quite likely to break at runtime. The only use case for using -Xtarget alone seems to be cross-compiling at your own risk for a newer JDK than the one used for the compilation.

For javac you get an error if you try to use -source and -target at the same time. That would be the best option IMHO but I'm not sure if this can be easily achieved with our current approach to parsing command line options.

That or a warning would be fine too, that could be checked in lazy val classfileVersion.

javac also emits a warning when you use -target explicitly but there is a mismatch with the current JDK version. Should we have it as well? (I guess we don't want to raise a warning for the default when compiling on JDK 9+)

Since we renamed it to -Xtarget it's an advanced option so I don't think a warning is worth it, but the documentation of Xtarget can mention that people should consider using release instead

prolativ · 2020-12-11T22:29:01Z

I've just found out that setting -release to something else than 8 or the current jdk leads to some errors in the ClassfileParser

Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 5167 while parsing annotations in /CD/java.base/java/lang/Class.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 5316 while parsing annotations in /CD/java.base/java/lang/Class.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1542 while parsing annotations in /9ABCDE/java.base/java/lang/Package.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1772 while parsing annotations in /BCDE/java.base/java/lang/reflect/Method.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1716 while parsing annotations in /BCDE/java.base/java/lang/reflect/Constructor.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1883 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1919 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1955 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 1991 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2027 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2063 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2099 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2135 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2171 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2207 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2243 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2279 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2315 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2351 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2387 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2423 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2459 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 2495 while parsing annotations in /BCDE/java.base/java/lang/reflect/Field.sig
Caught: java.lang.RuntimeException: bad constant pool index: 0 at pos: 4434 while parsing annotations in /BCDE/java.base/java/lang/invoke/MethodHandles.sig
24 warnings found

That might require further investigation

* A port of https://github.com/scala/scala/pull/6362/files with some improvements * When running scalac on JDK 9+ the -release option assures that code is compiled with classes specific to the release available on the classpath. This applies to classes from the JDK itself and from external jars. If the compilation succeeds, bytecode for the specified release is produced. * -target option gets renamed to -Xtarget. Using -release instead is preferred since -Xtarget sets the bytecode version without any checks so this might lead to producing code that breaks at runime * Fix parsing annotations in classfiles

prolativ · 2020-12-14T14:24:39Z

@smarter These errors were caused by an already existing bug in ClassfileParser. However fixing it didn't require much code so I integrated the fix (and an update to a test) into this PR. Have a look at that please

prolativ requested a review from smarter December 10, 2020 16:55

prolativ force-pushed the add-release-flag branch 2 times, most recently from 99f830e to 181b742 Compare December 10, 2020 22:08

smarter approved these changes Dec 11, 2020

View reviewed changes

smarter assigned prolativ Dec 11, 2020

prolativ force-pushed the add-release-flag branch from 181b742 to 47fef0b Compare December 11, 2020 22:25

prolativ force-pushed the add-release-flag branch from 47fef0b to 54125ce Compare December 14, 2020 14:19

smarter approved these changes Dec 15, 2020

View reviewed changes

smarter merged commit b3486ac into scala:master Dec 15, 2020

anatoliykmetyuk added the release-notes Should be mentioned in the release notes label Feb 10, 2021

prolativ mentioned this pull request Feb 16, 2021

Add RC1 blog article #11417

Merged

prolativ mentioned this pull request Oct 25, 2021

Consider minTargetVersion as always supported #13811

Merged

prolativ mentioned this pull request Mar 3, 2022

Align -release and -target with Scala 3 scala/bug#12543

Closed

mkurz mentioned this pull request Mar 10, 2022

Scala 3 playframework/play-ws#604

Merged

7 tasks

bjaglin mentioned this pull request Jun 12, 2023

Move to safer compatibility flag for javac scalacenter/scalafix#1786

Merged

Kordyjan added this to the 3.0.0 milestone Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #8634: Support -release option #10746

Fix #8634: Support -release option #10746

prolativ commented Dec 10, 2020

smarter left a comment

smarter Dec 11, 2020

smarter Dec 11, 2020

smarter Dec 11, 2020

prolativ Dec 11, 2020

smarter Dec 11, 2020 •

edited

Loading

prolativ Dec 11, 2020

smarter Dec 11, 2020

prolativ commented Dec 11, 2020 •

edited

Loading

prolativ commented Dec 14, 2020

	final class FileZipArchive(jpath: JPath, release: Option[String] = None) extends ZipArchive(jpath, release) {
	final class FileZipArchive(jpath: JPath, release: Option[String]) extends ZipArchive(jpath, release) {

Fix #8634: Support -release option #10746

Fix #8634: Support -release option #10746

Conversation

prolativ commented Dec 10, 2020

smarter left a comment

Choose a reason for hiding this comment

smarter Dec 11, 2020

Choose a reason for hiding this comment

smarter Dec 11, 2020

Choose a reason for hiding this comment

smarter Dec 11, 2020

Choose a reason for hiding this comment

prolativ Dec 11, 2020

Choose a reason for hiding this comment

smarter Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

prolativ Dec 11, 2020

Choose a reason for hiding this comment

smarter Dec 11, 2020

Choose a reason for hiding this comment

prolativ commented Dec 11, 2020 • edited Loading

prolativ commented Dec 14, 2020

smarter Dec 11, 2020 •

edited

Loading

prolativ commented Dec 11, 2020 •

edited

Loading