Skip to content

chore: use language import instead of -Ycompile-scala2-library #23082

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hamzaremmal
Copy link
Member

This PR adds a new language import (scala.language.2) to replace the -Ycompile-scala2-library flag.
This language import should only be used in the stdlib (package scala). To enforce this, the language import is private[scala] but we still need to enforce it (#23081).

For now, I've allowed the -source 2 option but this will be disallowed once we add the source files of the stdlib in this repository.

Comment on lines -1540 to -1541
if ctx.settings.YcompileScala2Library.value then
()
Copy link
Member Author

Choose a reason for hiding this comment

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

Since scala.language.2 lives in the patches, we will need to patch the denotations to gain access to it. Otherwise, typer complains because the member is not found.

@hamzaremmal hamzaremmal requested a review from natsukagami April 30, 2025 20:23
@hamzaremmal hamzaremmal removed the request for review from natsukagami April 30, 2025 20:42
@hamzaremmal hamzaremmal force-pushed the compile-as-scala2 branch 2 times, most recently from 6aaf63d to 3aed10f Compare April 30, 2025 21:18
@@ -17,6 +17,7 @@ enum SourceVersion:
case `3.7-migration`, `3.7`
case `3.8-migration`, `3.8`
// !!! Keep in sync with scala.runtime.stdlibPatches.language !!!
case `2`
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this one leak into the -source setting choices that can be specified by users?

I guess that the main goal of this PR is to restrict access to special mode of compilation Scala 2 standard library so that it cannot be used in any other project. Is there a plan to compile any other sources then Scala 2 stdlib? If no, then maybe special marker class added to sources would be enough to to check at compile time (eg. using requiredClass method) that -Ycompile-scala2-library is not used in foreign scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't this one leak into the -source setting choices that can be specified by users?

As it is right now, yes. And I actually use it to make this work until I add the imports manually. But when this is done, it will be forbidden in -source. We can disallow the option by adding "2" here:

val illegalSourceVersionNames = List("3.1-migration", "never").map(_.toTermName)

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that the main goal of this PR is to restrict access to special mode of compilation Scala 2 standard library so that it cannot be used in any other project.

Actually, this is not the primary goal of this PR. We will be adding the source files of the scala2 library in the same folder as the scala3 library and we need a way to still behave as scala2 for those specific files. A flag affects all the files while an import can affect on specific compilation units.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't all files of single sbt project part of single compilation unit beside macros? I always thought there is no effective difference between the flag and import, but maybe I've missed something

@hamzaremmal
Copy link
Member Author

hamzaremmal commented May 1, 2025

We will revert commit: 43e45e7 when we merge this PR

@hamzaremmal hamzaremmal force-pushed the compile-as-scala2 branch 2 times, most recently from 11b9a52 to 43e45e7 Compare May 1, 2025 16:50
@hamzaremmal hamzaremmal force-pushed the compile-as-scala2 branch from 43e45e7 to a10622a Compare May 2, 2025 11:13
@hamzaremmal hamzaremmal added this to the 3.8.0 milestone May 5, 2025
@hamzaremmal
Copy link
Member Author

I'm renaming the object from 2 to 2.13

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

Successfully merging this pull request may close these issues.

3 participants