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

Deprecation warnings for old syntax: var x = _ #18821

Merged

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Nov 1, 2023

First part #18861

  • In 3.4 we emit the deprecation warning
  • In future we emit we make this syntax an error
  • Add patch. Not ideal because we need to use the full path of uninitialized
//> using options -source future
var x: Int = _ // error
//> using options -rewrite -source 3.4-migration
- var x: Int = _
+ var x: Int = scala.compiletime.uninitialized

@nicolasstucki nicolasstucki self-assigned this Nov 1, 2023
@nicolasstucki nicolasstucki changed the title Make uninitialized var syntax a migration warning Future migration warning for uninitialized var x = _ Nov 2, 2023
@nicolasstucki

This comment was marked as outdated.

@nicolasstucki nicolasstucki marked this pull request as ready for review November 2, 2023 10:10
@nicolasstucki
Copy link
Contributor Author

In this week's Dotty meeting we decided to do the rewrite as it is implemented here.

@nicolasstucki nicolasstucki removed the request for review from odersky November 7, 2023 10:30
@nicolasstucki nicolasstucki changed the title Future migration warning for uninitialized var x = _ Deprecation warnings for old syntax: var x = _ Nov 7, 2023
@nicolasstucki nicolasstucki force-pushed the uninitialized-migration-warning branch 4 times, most recently from bb3e2f9 to cfe81d4 Compare November 7, 2023 12:17
@@ -578,7 +578,8 @@ object Build {
lazy val commonDottyCompilerSettings = Seq(
// Note: bench/profiles/projects.yml should be updated accordingly.
Compile / scalacOptions ++= Seq("-Yexplicit-nulls", "-Ysafe-init"),

// Use source 3.3 to avoid fatal migration warnings on scalajs-ir
scalacOptions ++= Seq("-source", "3.3"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sjrd where can I add this options to enable them only when compiling the scalajs-ir?
See https://github.com/lampepfl/dotty/actions/runs/6784134103/job/18439824856

Copy link
Member

Choose a reason for hiding this comment

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

I don't think you can. The scalajs-ir sources are built as part of scala3-compiler itself, so they must share their flags.

@nicolasstucki nicolasstucki added this to the 3.4.0 milestone Nov 7, 2023
@nicolasstucki nicolasstucki added the release-notes Should be mentioned in the release notes label Nov 8, 2023
@nicolasstucki nicolasstucki force-pushed the uninitialized-migration-warning branch 2 times, most recently from 252b4b6 to b80e474 Compare November 8, 2023 10:13
* In `3.4` we emit the deprecation warning
* In `future` we emit we make this syntax an error
* Add patch. Not ideal because we need to use the full path of `uninitialized`
@nicolasstucki nicolasstucki merged commit 6b12bf0 into scala:main Nov 10, 2023
18 checks passed
@nicolasstucki nicolasstucki deleted the uninitialized-migration-warning branch November 10, 2023 15:32
@ckipp01
Copy link
Member

ckipp01 commented Dec 4, 2023

I know this is pretty after-the-fact, but this change did remove the unique error id that was associated with this error in the past, which was sort of a bummer. Since now if there was any automated tooling that could try to fix this that was utilizing that before, it's not longer present.

Screenshot 2023-12-04 at 13 43 50

In the future as these are done, could they be done in a way that still retains the unique error id or adds a new one if it now has a new meaning. If not all the work that went into ensuring that these unique ids were there across the entire ecosystem was sort of for nothing.

EDIT: I know this was probably not done intentionally. I just bring it up since it'd be great to have something that prevented this sort of thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants