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

support -release option #8634

Closed
ohze opened this issue Mar 30, 2020 · 19 comments
Closed

support -release option #8634

ohze opened this issue Mar 30, 2020 · 19 comments

Comments

@ohze
Copy link

ohze commented Mar 30, 2020

scalac has -release option
dotc should support this option too.

@He-Pin
Copy link

He-Pin commented Mar 30, 2020

Well, so dotc will at least require Java 8 right?

@smarter
Copy link
Member

smarter commented Apr 6, 2020

Well, so dotc will at least require Java 8 right?

Yes.

@anatoliykmetyuk anatoliykmetyuk removed their assignment Nov 24, 2020
@prolativ
Copy link
Contributor

@smarter how should -release option relate to -target (which seems to be missing in scala2)?

@prolativ
Copy link
Contributor

Thanks, I was looking for it in another file. But still they look quite similar to me

@smarter
Copy link
Member

smarter commented Nov 25, 2020

-target determines the version number we set in the classfiles we emit, with javac, --release X implies --target X, but with scalac it looks like they're independent. I think either is fine but what javac does makes more sense here.

@prolativ
Copy link
Contributor

Corresponding PR in scala2 repo: scala/scala#6362

@prolativ
Copy link
Contributor

prolativ commented Dec 9, 2020

@smarter what should be the supported values for -release and -target actually? It looks like currently the accepted values in scala 2 are from 6 to 9 for -release and from 8 to 12 for -target but that's probably because someone forgot to update the values for -release while changing that for -target (at least it looks like this when I look at the history of commits) because they should be the same (unless there's something that I've fundamentally misunderstood about this issue).
But in scala 3 only jvm-1.8 and jvm-9 are allowed as -target (also note that scala 2 treats 8, jvm-8 and jvm-1.8 as equivalents while scala 3 doesn't seem to give this freedom). However internally we still seem to support older JVMs (see BCodeIdiomatic.scala)

  lazy val classfileVersion: Int = ctx.settings.target.value match {
    case "jvm-1.5"     => asm.Opcodes.V1_5
    case "jvm-1.6"     => asm.Opcodes.V1_6
    case "jvm-1.7"     => asm.Opcodes.V1_7
    case "jvm-1.8"     => asm.Opcodes.V1_8
    case "jvm-9"       => asm.Opcodes.V9
  }

@smarter
Copy link
Member

smarter commented Dec 9, 2020

We should support 8 up to the latest release supported by asm.Opcodes. Versions before 8 never worked with dotty even if some dead code might mention them.

@prolativ
Copy link
Contributor

prolativ commented Dec 9, 2020

Should we also support alternatives naming schemes (8, jvm-8, jvm-1.8 etc.)?

@smarter
Copy link
Member

smarter commented Dec 9, 2020

Not worth it I think, let's just keep the version number

@prolativ
Copy link
Contributor

prolativ commented Dec 9, 2020

Actually I'm wondering weather having -target and -release as separate options makes sense for scala at all. In java one will be using -release most of the time but still -source and -target are needed when one wants to set different values for them. In scala -source and -target are quite independent (-source is the version of scala and -target is the version of jdk so they won't be the same). In the implementation from scala 2 -release seems just to make code be compiled with class file dependencies specific to the given release (either taken directly from the jdk or from external jars) but does not influence the version of produced bytecode. So with this approach setting -release without -target set to the same value doesn't seem to give the expected result and might be misleading (people might set only -release without -target thinking it will work as in javac). Would it then make sense to instead of adding a new compiler option just modify how -target works to always have the result of -release (when running the compiler on jdk >= 9; for jdk 8 the old behaviour would be kept since -release is not supported there)?

@smarter
Copy link
Member

smarter commented Dec 9, 2020

I think -release should also set -target (this is what javac does), I'm more hesitant about merging both flags (especially since as you mention it would have to do different things under jdk 8), one option would be to keep -target but rename it to -Xtarget to encourage people to use -release instead.

@prolativ
Copy link
Contributor

prolativ commented Dec 9, 2020

Another discrepancy is that the default value of -target in javac seems to be the same as the version of jdk which is used to run the compilation (at least for jdk >= 8). In our case it's currently fixed to jdk 8. Should we mimic the behaviour of javac in terms to this?

@smarter
Copy link
Member

smarter commented Dec 9, 2020

In our case it's currently fixed to jdk 8. Should we mimic the behaviour of javac in terms to this?

I would rather not since Scala has never behaved that way (it always emits bytecode compatible with the lowest supported JDK).

@prolativ
Copy link
Contributor

It looks like in scala 2 if you run scalac you can set -target to a version greater than the jdk you're currently using. Should we disallow this in general? Or should it be disallowed only for -release but allowed for -Xtarget as for the latter no additional checks for compatibility with the selected jdk would be performed?

@smarter
Copy link
Member

smarter commented Dec 10, 2020

Or should it be disallowed only for -release but allowed for -Xtarget as for the latter no additional checks for compatibility with the selected jdk would be performed?

Makes sense to me.

prolativ added a commit to prolativ/dotty that referenced this issue Dec 10, 2020
* 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
@prolativ
Copy link
Contributor

prolativ commented Dec 10, 2020

Another thing that came to my mind: if we don't set -release explicitly (so, from the default, bytecode for JDK 8 will be emitted) but scalac is run on JDK 9+, should we set the classpath as for -release 8 (unless -Xtarget i set)?

@smarter
Copy link
Member

smarter commented Dec 10, 2020

No, Scala has never behaved that way and I don't think it's worth changing that.

prolativ added a commit to prolativ/dotty that referenced this issue Dec 10, 2020
* 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
prolativ added a commit to prolativ/dotty that referenced this issue Dec 10, 2020
* 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
prolativ added a commit to prolativ/dotty that referenced this issue Dec 10, 2020
* 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
prolativ added a commit to prolativ/dotty that referenced this issue Dec 11, 2020
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants