Allow either -Xsource:3 (for preparing to switch to 3) or -Xsource:3-cross (for crossbuilding on 2 and 3)#10573
Conversation
6ef2b2e to
49942b7
Compare
|
@lrytz @SethTisue I'm not sure distinguishing "warning 3" from "behavior 3" is the right direction. That is because of Lukas's recent remark, and also the ticket about case class apply access. I saw an early comment from Seth about "the behavior change is only under -Xsource:3", so I think "behind a flag" meant "anything goes". In the new regime, there is regular 2.13 which has some deprecations, Changes are more-or-less listed under |
|
To me this looks good. Thank you for all the attention to detail, and for the help text! @SethTisue wdyt? |
|
I had forgotten that this proceeded. I see the point here was to split migration help over the two settings. There's a different branch |
|
I see that even if a deprecation is issued at 2.13, at 3 a (possibly more menacing) warning is issued as The possible exception to this rule is "benign syntax", such as accepting Edit: ...and the warning issued at 3 is configured at Somewhat optimistically, I'll try to audit that the 4 config are reflected in tests: 2.13., 3+migration, 3, 3-X. The code switches between 2, 3, 3-X. It's not reasonable to warn about how parser constructs interpolations, unless it emits a warning that is only summarized: at the end, it could say you used string interpolation, which has new semantics etc; assuming it's too much work to check for shadowing |
c00a987 to
32d36c4
Compare
bec5686 to
026cc00
Compare
lrytz
left a comment
There was a problem hiding this comment.
@som-snytt I rebased it (without changing your commits) and added one commit. I added 5 tests in the various categories (source3neg, source3Xneg, deprecationsForSource3, source3run, source3Xrun), it just made it easier for me to align things. They mostly duplicate existsing tests which I didn't remove.
For all cases where semantics change under source:3-X, I made sure there's a warning under source:3 and no warning under 3-X.
Let me know if that looks fine.
|
@lrytz thanks for not asking me to revisit this PR. Your clever use of attachments cleans up nicely. Thanks for the improved mnemonic for I see you quashed a couple of messages under "cross building" is when you get mad at |
|
|
maybe |
|
@SethTisue it's actually any version string that compares greater than "3.0.0", which includes "3-X" where the (Entrance to warren: I think |
The advantage of a magic string is that it gives something a fixed name so we — and not just we, but the whole community — can refer to it conversation, documentation, PR review, blog posts, etc. I think it should have one name. (And I think we can find a better one than |
|
Since I never get what I love, you or Lukas at your behest may invent something, or invent a version string that always compares last, namely, Now that I said it, I also love that idea. Then we can also say the tag line, " |
|
I see from a glance at the source that |
026cc00 to
b275b38
Compare
|
@SethTisue this is ready for review |
b275b38 to
b353f1d
Compare
|
I guess you can click on it, too. The first weird error: edit: scrolling right |
|
If this were merged at Thanksgiving, it wouldn't have weird build errors after MLK, Jr Day. (U.S. reference points.) lrytz just re-rebases on top, but I am way too lazy slash process-averse to do that. |
|
My main concern here is the naming. (Which I did ponder repeatedly during my long silence... sorry about that...) I do want:
I don't want:
I suggest we keep So then this PR would simply add @som-snytt @lrytz wdyt? |
|
|
|
Any label is OK by me. Dotty has However, "Gee that's a dumb name for an option" does not impede the dotty team. Nor should it us. |
|
Let's see what Lukas thinks. The vibe I'm going for here is "There's |
|
We did change everything around. There used to be a mix of warnings and behavior changes until Lukas imposed Swiss tidiness. The user complaint was "don't warn about the behavior change I asked for", so But whatever option names are chosen now is OK, people will figure it out. |
Okay, I see what you mean. Yes, I acknowledge that I'm idealizing the coherence of the prior state a bit :-) |
|
The most common use case for It seems Scala 2 is anyway different from dotty
I don't know if |
-Xsource:3 (for preparing to move to 3) and -Xsource:cross (for crossbuilding on 2 and 3)
|
I'd be okay with |
-Xsource:3 (for preparing to move to 3) and -Xsource:cross (for crossbuilding on 2 and 3)-Xsource:3 (for preparing to move to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)
(I just suggested at https://users.scala-lang.org/t/scala-3-porting-experiences/9753/5 that perhaps Scala 3 ought to also offer a flag specifically for crossbuilt projects, since 3.4 will now warn by default about tons of stuff that isn't actionable in crossbuilt projects.) |
-Xsource:3 (for preparing to move to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)-Xsource:3 (for preparing to switch to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)
Add documentation to `-Xsource:help`. -Xsource:3migrate is an alias for `-Xsource:3`, it enables fatal warnings but doesn't change semantics. -Xsource:3cross adpots Scala 3 semantics for certain constructs and skips the warning, for example modifiers of case class apply / copy methods. Warnings for things that should be addressed because they are deprecated / unsupported in Scala 3 remain.
b353f1d to
3950943
Compare
|
🎉 |
|
Anyone else notice that the original |
|
@som-snytt @lrytz I just did an edit pass over the PR description — y'all might want to take another look as well. |
-Xsource:3 (for preparing to switch to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)-Xsource:3 (for preparing to switch to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)
-Xsource:3 (for preparing to switch to 3) and -Xsource:3-cross (for crossbuilding on 2 and 3)-Xsource:3 (for preparing to switch to 3) or -Xsource:3-cross (for crossbuilding on 2 and 3)
|
I see that The help text mentions Edit: added a few words above. Now, I would like to go grocery shopping, please, so that I can squeeze in some exercise after. |
Documentation
A new section of the Scala 3 Migration Guide documents
-Xsource:3and-Xsource:3-cross: https://docs.scala-lang.org/scala3/guides/migration/tooling-scala2-xsource3.htmlDescription
-Xsource:3remains intended for users preparing to upgrade a codebase from 2 to 3. It enables migration warnings but doesn't change semantics. By default, the warnings are configured to emit errors; to report them only as warnings, use something like the following:-Wconf:cat=scala3-migration:w.The new
-Xsource:3-crossis intended for users crossbuilding the same code for 2 and 3. It adopts Scala 3 semantics for certain constructs and skips warnings that aren't actionable in crossbuilt code. But it still warns for things that should be addressed because they are deprecated / unsupported in Scala 3.Important: When upgrading to Scala 2.13.13 from an earlier version, consider switching from
-Xsource:3to-Xsource:3-cross, if you are crossbuilding between Scala 2 and 3, or if you were relying on particular behaviors of-Xsource:3. Some behaviors that were formerly enabled by-Xsource:3are now only enabled by-Xsource:3-cross, for example around return type inference for inherited methods.Documentation of what specifically is enabled by these flags is in the new
-Xsource:help, as well as the new doc page.Also fixes scala/bug#12886 by un-deprecating package object inheritance.