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

Changing class parameters doesnt recompile dependent code #1334

Closed
sake92 opened this issue Feb 13, 2024 · 9 comments · Fixed by scala/scala3#19911
Closed

Changing class parameters doesnt recompile dependent code #1334

sake92 opened this issue Feb 13, 2024 · 9 comments · Fixed by scala/scala3#19911
Labels
area/scala3 Zinc issue specific to Scala 3 area/under_compilation Zinc does not pick up compilation when needed

Comments

@sake92
Copy link

sake92 commented Feb 13, 2024

steps

problem

Hello still compiles.

expectation

Hello should not compile.

notes

Seems to be scala 3 related. It works with 2.13 for example

Original issue in Metals: scalameta/metals#6112

@SethTisue
Copy link
Member

fyi @Gedochao, as the root cause may be in the Dotty repo

@sake92
Copy link
Author

sake92 commented Mar 2, 2024

Hmm, I noticed another similar issue..
I moved a class to another folder, even changed the package, and it did not fail compilation.. Really weird

@sake92
Copy link
Author

sake92 commented Mar 9, 2024

It's not a Mill issue btw, I've added sbt too, same issue.
What I didn't realize at the time is that the code successfully compiles, but throws NoSuchMethodError !

PS C:\projects\sake\metals-mill-scala3-class-params-repro> mill scalaHelloWorld.run
[36/49] scalaHelloWorld.compile
[info] compiling 1 Scala source to C:\projects\sake\metals-mill-scala3-class-params-repro\out\scalaHelloWorld\compile.dest\classes ...
[info] done compiling
[49/49] scalaHelloWorld.run
Exception in thread "main" java.lang.NoSuchMethodError: 'void example.MyService.<init>(java.lang.String)'
        at example.Hello$.<clinit>(Hello.scala:5)
        at example.Hello.main(Hello.scala)
1 targets failed
scalaHelloWorld.run subprocess failed

@eed3si9n
Copy link
Member

eed3si9n commented Mar 9, 2024

@sake92 Would something like the following be an example?

class MyService(str: String, i: Int)

Like Seth suggested, likely this is more to do with the compiler bridge issue, and not specific to the build tools that calls into Zinc like Mill or sbt.

@eed3si9n eed3si9n added area/under_compilation Zinc does not pick up compilation when needed area/scala3 Zinc issue specific to Scala 3 labels Mar 9, 2024
@eed3si9n
Copy link
Member

eed3si9n commented Mar 9, 2024

val service = MyService("fssdfs")

My guess is that the new new-less syntax is failing to register dependency relationship to the MyService constructor, so when the constructor changed, Hello object isn't recompiled.

@eed3si9n
Copy link
Member

eed3si9n commented Mar 9, 2024

Actually adding new didn't change the behavior:

> hello/run
[info] compiling 1 Scala source to /private/tmp/metals-mill-scala3-class-params-repro/scalaHelloWorld/target/scala-3.3.1/classes ...
[info] running example.Hello
Exception in thread "sbt-bg-threads-13" java.lang.NoSuchMethodError: 'void example.MyService.<init>(java.lang.String)'
	at example.Hello$.<clinit>(Hello.scala:5)
	at example.Hello.main(Hello.scala)

@eed3si9n
Copy link
Member

eed3si9n commented Mar 9, 2024

I suggest reporting this to https://github.com/scala/scala3/issues.

@sake92
Copy link
Author

sake92 commented Mar 9, 2024

Thanks for checking @eed3si9n .
I'll report to the scala3 repo.

bishabosha added a commit to scala/scala3 that referenced this issue Mar 17, 2024
Fixes #19910
Fixes sbt/zinc#1334

## Problem
Scala 3 compiler registers special `zincMangledName` for constructors
for the purpose of incremental compilation. Currently the
`zincMangledName` contains the package name, which does not match the
use site tracking,
thereby causing undercompilation during incremental compilation after a
ctor change, like adding a parameter.

There is an existing scripted test, but coincidentally the test class
does NOT include packages, so the test ends up passing.

## Solution
1. This PR reproduces the issue by adding package name to the test.
2. This also fixes the problem by changing the `zincMangedName` to
`sym.owner.name ++ ";init;"`.

## Note about zincMangedName
`zincMangedName` in general is a good idea, which adds the class name
into otherwise common name `<init>`.
By making the name more unique it prevents overcompilation when one of
the ctors changes in a file.
@Friendseeker
Copy link
Member

Fixed by scala/scala3#19911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scala3 Zinc issue specific to Scala 3 area/under_compilation Zinc does not pick up compilation when needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants