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

Consistent naming of output-version settings #14606

Merged
merged 1 commit into from Mar 2, 2022

Conversation

prolativ
Copy link
Contributor

@prolativ prolativ commented Mar 2, 2022

Following the scheme https://contributors.scala-lang.org/t/improving-scala-3-forward-compatibility/5298/22
rename compiler settings:
-Yscala-release -> -scala-output-version
-release -> -java-output-version
-Xtarget -> -Xunchecked-java-output-version
(leaving old names as aliases for compatibility)

@prolativ prolativ requested a review from smarter March 2, 2022 12:56
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think renaming Xtarget is worth it, it's an advanced option anyway and the name as is makes it clearer it matches the semantics of -target in java. Otherwise LGTM.

@@ -103,7 +103,9 @@ trait CommonScalaSettings:
val pageWidth: Setting[Int] = IntSetting("-pagewidth", "Set page width", ScalaSettings.defaultPageWidth, aliases = List("--page-width"))
val silentWarnings: Setting[Boolean] = BooleanSetting("-nowarn", "Silence all warnings.", aliases = List("--no-warnings"))

val release: Setting[String] = ChoiceSetting("-release", "release", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version.", ScalaSettings.supportedReleaseVersions, "", aliases = List("--release"))
val javaOutputVersion: Setting[String] = ChoiceSetting("-java-output-version", "version", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version. Corresponds to -release flag in javac.", ScalaSettings.supportedReleaseVersions, "", aliases = List("-release", "--release"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make a PR to Scala 2 to also rename -release there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should rename it for scala 2 as the semantics of -release and -target are slightly different there #8634 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that would also require changing the scala 2 flag to also set the target, IMO the current behavior is an oversight.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scala/bug#12543

without over-promising, maybe I'll take the opportunity to understand the issues and ergonomics.

@prolativ
Copy link
Contributor Author

prolativ commented Mar 2, 2022

Regarding -Xtarget I think it's good to keep symmetry anyway.

@prolativ prolativ merged commit db7872e into scala:main Mar 2, 2022
@prolativ prolativ deleted the rename-output-version-settings branch March 4, 2022 15:25
@bishabosha bishabosha added the backport:done This PR was successfully backported. label Mar 9, 2022
@mkurz mkurz mentioned this pull request Mar 10, 2022
7 tasks
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
som-snytt pushed a commit to scala/scala that referenced this pull request Jan 13, 2024
Following on from #9982 and
discussion in https://github.com/scala/scala/pull/9982/files#r1434172490,
this change enables `-java-output-version` as an alternative name for
the `-release` flag in Scala 2, as in Scala 3 `-java-output-version` is
the preferred flag name (see scala/scala3#14606 ),
with the alias of `-release` still permitted.

Correspondingly, `-Xunchecked-java-output-version` is also included as an
alternative for `-target`, though note use of this flag is discouraged
in favour of `-java-output-version`/`-release` for Java 9 and above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants