Skip to content

Conversation

tgodzik
Copy link
Collaborator

@tgodzik tgodzik commented Jul 21, 2025

No description provided.

@tgodzik tgodzik force-pushed the update-evaluator branch 4 times, most recently from c1b4f8f to 8a31772 Compare July 22, 2025 08:01
@tgodzik tgodzik marked this pull request as ready for review July 22, 2025 08:25
@adpi2
Copy link
Member

adpi2 commented Jul 22, 2025

@tgodzik thank you for pushing this forward. I'll take a look at it soon.

@adpi2 adpi2 self-requested a review July 22, 2025 08:43
@tgodzik tgodzik force-pushed the update-evaluator branch from 8a31772 to 25a5feb Compare July 22, 2025 10:50
@tgodzik
Copy link
Collaborator Author

tgodzik commented Jul 24, 2025

@adpi2 would you be able to take a look? We would want to have it ready in time for the next Scala release. It's totally ok if you don't have time.

Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Also, could you add a test to make sure that we can invoke the Scala 3.7 expression compiler?

Comment on lines 113 to 122
val expressionCompilerArtifact =
s"${BuildInfo.expressionCompilerName}_${scalaVersion.value}"
val (expressionCompilerOrg, expressionCompilerArtifact, expressionCompilerVersion) =
if (scalaVersion.isScala3 && scalaVersion.minor >= 7) {
("org.scala-lang", s"scala3-compiler_3", scalaVersion.value)
} else {
(BuildInfo.organization, s"${BuildInfo.expressionCompilerName}_${scalaVersion.value}", BuildInfo.version)
}
val expressionCompilerDep = Dependency(
coursier.Module(Organization(BuildInfo.organization), ModuleName(expressionCompilerArtifact)),
BuildInfo.version
coursier.Module(Organization(expressionCompilerOrg), ModuleName(expressionCompilerArtifact)),
expressionCompilerVersion
Copy link
Member

Choose a reason for hiding this comment

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

NIT: I think the code would be simpler to understand if ScalaInstance.expressionCompilerJar becomes a Option[Library].

fetchScala3 would become a if else expression. If 3.7, we resolve the compiler and there is no expressionCompilerJar, else we resolve the expressionCompilerArtifact as before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it, tough we still fetch the compiler jars, so I had to leave the code more or less the same. Otherwise we would just be duplicating things

@tgodzik
Copy link
Collaborator Author

tgodzik commented Jul 25, 2025

Also, could you add a test to make sure that we can invoke the Scala 3.7 expression compiler?

I added a test that invokes 3.7.2-RC2, or did you mean something else?

@tgodzik tgodzik requested a review from adpi2 July 25, 2025 11:23
@WojciechMazur
Copy link

Hi @adpi2 , when you get a moment, could you please take a look at this PR agin?
It might be blocking the upcoming Scala compiler release. Would appreciate your input to help move it forward. Thanks in advance!

@tgodzik
Copy link
Collaborator Author

tgodzik commented Jul 29, 2025

Hi @adpi2 , when you get a moment, could you please take a look at this PR agin? It might be blocking the upcoming Scala compiler release. Would appreciate your input to help move it forward. Thanks in advance!

I went ahead with a new release, so we should be good for now

Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.

That looks great!

@tgodzik tgodzik force-pushed the update-evaluator branch from 875c277 to ca0672f Compare July 31, 2025 11:53
@tgodzik tgodzik merged commit 858e0bc into scalacenter:main Jul 31, 2025
9 checks passed
@tgodzik tgodzik deleted the update-evaluator branch July 31, 2025 13:08
tgodzik added a commit that referenced this pull request Sep 1, 2025
improvement: Use expression evaluator from the compiler for 3.7.0+
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants