Skip to content

[backport] Align -release and -target per Scala 3#10089

Merged
lrytz merged 1 commit intoscala:2.12.xfrom
som-snytt:backport/release-target
Jul 28, 2022
Merged

[backport] Align -release and -target per Scala 3#10089
lrytz merged 1 commit intoscala:2.12.xfrom
som-snytt:backport/release-target

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Jul 26, 2022

Backport #9982

Reminder by scala/bug#12625

Omits fixes to arg processing. -release:8 but not -release 8.

➜  ~ skala -target:jvm-1.5
Welcome to Scala 2.12.17-20220726-143638-9f37fd6 (OpenJDK 64-Bit Server VM, Java 18.0.1.1).
Type in expressions for evaluation. Or try :help.

scala> 42
warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
res0: Int = 42

scala> :quit
➜  ~ skala -target:jvm-11
Welcome to Scala 2.12.17-20220726-143638-9f37fd6 (OpenJDK 64-Bit Server VM, Java 18.0.1.1).
Type in expressions for evaluation. Or try :help.

scala> 42
warning: -target is deprecated: Use -release instead to compile against the correct platform API.
res0: Int = 42

The hack to allow one shot at setting deprecation on the settings incurs a limitation:

➜  ~ skala
Welcome to Scala 2.12.17-20220726-143638-9f37fd6 (OpenJDK 64-Bit Server VM, Java 18.0.1.1).
Type in expressions for evaluation. Or try :help.

scala> 42
res0: Int = 42

scala> :replay -target:jvm-1.5
Replaying: 42
[init] warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
res0: Int = 42


scala> :replay -target:jvm-11
Replaying: 42
[init] warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
res0: Int = 42

Unexpectedly, or perhaps expectedly, reset does not reset enough:

scala> :reset -target:jvm-11
Resetting interpreter state.
Forgetting this session history:

42


scala> 42
[init] warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
warning: -target is deprecated: -target:5 is deprecated, forcing use of 8
res0: Int = 42

@scala-jenkins scala-jenkins added this to the 2.12.17 milestone Jul 26, 2022
@som-snytt som-snytt mentioned this pull request Jul 26, 2022
Merged
@som-snytt som-snytt marked this pull request as ready for review July 26, 2022 19:45
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Jul 26, 2022
lrytz
lrytz approved these changes Jul 28, 2022
View reviewed changes
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

👍 thanks!

@lrytz lrytz merged commit 25648c2 into scala:2.12.x Jul 28, 2022
@som-snytt som-snytt deleted the backport/release-target branch July 28, 2022 08:24
@lrytz lrytz mentioned this pull request Jul 28, 2022
Merged
@lrytz
Copy link
Member

lrytz commented Jul 29, 2022 •
edited
Loading

... is there an easy way to keep accepting -release N (in addition to -release:N)?

@som-snytt
Copy link
Contributor Author

som-snytt commented Jul 29, 2022

I was daunted by whatever change I made on the other PR. Possibly I am lazy, or underpaid.

@lrytz
Copy link
Member

lrytz commented Jul 29, 2022

Maybe just #10099

@lrytz lrytz mentioned this pull request Jul 29, 2022
Merged
@lrytz
Copy link
Member

lrytz commented Aug 15, 2022 •
edited
Loading

I worry about -release N automatically setting -target N on 2.12. We don't have the fix for scala/scala-dev#408 in 2.12, so using anything but -target 8 on 2.12 is basically always going to crash at run-time.

➜ sandbox qs -release 11
Welcome to Scala 2.12.17-20220801-023710-5ff747f (OpenJDK 64-Bit Server VM, Java 17.0.1).
Type in expressions for evaluation. Or try :help.

scala> trait T { val x = 42 }; class C extends T
defined trait T
defined class C

scala> new C
java.lang.IllegalAccessError: Update to non-static final field C.x attempted from a different method (T$_setter_$x_$eq) than the initializer method <init>
  at C.T$_setter_$x_$eq(<console>:11)
  at T.$init$(<console>:11)
  ... 30 elided

It's probably fine on 2.13, hoping that we don't have any more bugs like #9915 in there.

@lrytz lrytz mentioned this pull request Aug 15, 2022
Merged
@SethTisue
Copy link
Member

SethTisue commented Aug 31, 2022 •
edited
Loading

I worry about -release N automatically setting -target N on 2.12

Note that this was taken care of over at #10109, which makes it so that 2.12 always emits Java 8 bytecode.

@SethTisue SethTisue removed the release-notes worth highlighting in next release notes label Aug 31, 2022
@SethTisue
Copy link
Member

SethTisue commented Aug 31, 2022

Note that I've removed the release-notes label here, but left it on #10109. It's confusing for users when a single conceptual change is documented across multiple PRs, so let's make sure #10109 has a good PR description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lrytz lrytz lrytz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.12.17

Development

Successfully merging this pull request may close these issues.

4 participants

@som-snytt @lrytz @SethTisue @scala-jenkins