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

Zinc invalidation when nested inline method changes #11861

Closed
adpi2 opened this issue Mar 23, 2021 · 4 comments · Fixed by #12931
Closed

Zinc invalidation when nested inline method changes #11861

adpi2 opened this issue Mar 23, 2021 · 4 comments · Fixed by #12931

Comments

@adpi2
Copy link
Member

adpi2 commented Mar 23, 2021

Compiler version

Latest nightly:

scalaVersion :=  "3.0.0-RC2-bin-20210320-0195dff-NIGHTLY"

Minimized code

// Show.scala
case class Show(name: String)
  
object Show:
  inline given Show = foo
  
  private inline def foo: Show = Show("foo")
// App.scala
@main def app() =
  println(summon[Show].name)

Output

  1. compile and run
sbt:dotty-bug> run
[info] compiling 2 Scala sources to /home/piquerez/lampepfl/dotty-bug/target/scala-3.0.0-RC2/classes ...
[info] running app 
foo
[success] Total time: 0 s, completed Mar 23, 2021, 2:55:05 PM
  1. change the implimation of the private inline foo method
 object Show:
   inline given Show = foo
   
-  private inline def foo: Show = Show("foo")
+  private inline def foo: Show = Show("bar")
  1. compile and run again
sbt:dotty-bug> run
[info] compiling 1 Scala source to /home/piquerez/lampepfl/dotty-bug/target/scala-3.0.0-RC2/classes ...
[info] running app 
foo
[success] Total time: 0 s, completed Mar 23, 2021, 2:57:21 PM

The app.scala file is not invalidated and thus not recompiled.
The produced program should print "bar" instead of "foo".

Expectation

The app.scala file should be recompiled.

@adpi2
Copy link
Member Author

adpi2 commented Mar 23, 2021

Actually Show does not need to be resolved implicitly.

I have the same behavior with:

case class Show(name: String)
  
object Show:
  inline def foo: Show = bar
  inline def bar: Show = Show("foo")
@main def app() =
  println(Show.foo.name)

@smarter
Copy link
Member

smarter commented Mar 23, 2021

Currently, the API hash of an inline method contains a hash of the body of the method: https://github.com/lampepfl/dotty/blob/c0b3fde1dbe9a8ef3b87ddfaa873ea945beafa8c/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala#L588-L682 I think we need to go further and also recursively include the bodies of inline methods called from this method, keeping in mind that this requires maintaining a set of seen methods to make sure we don't recurse infinitely. (This is a bit annoying since it means we need to traverse the same inline method multiple times every time we see it called from some other inline method, but any solution that tries to reuse hashes is bound to be very complicated because we can have mutually-recursive inline methods that can even be defined in separate compilation units)

@bishabosha
Copy link
Member

so there is no issue with inlining being later now - we can still access the bodies

@bishabosha bishabosha added this to Minimized in Spree Jun 22, 2021
@adpi2 adpi2 moved this from Minimized to Assigned in Spree Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 22, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 24, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 24, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 25, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 25, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 25, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2021
@Iltotore
Copy link
Contributor

Iltotore commented Sep 6, 2021

I suspect this issue to be the cause of deffered inline methods when changing the implementation then recompiling but works when recompiling the entire project.

bishabosha added a commit to dotty-staging/dotty that referenced this issue Nov 19, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Nov 19, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 2, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 2, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 6, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 6, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 15, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 15, 2021
bishabosha added a commit that referenced this issue Dec 22, 2021
Fix #11861 - hash nested calls to inline definitions
@TheElectronWill TheElectronWill moved this from Assigned to Fixed in Spree Mar 18, 2022
olsdavis pushed a commit to olsdavis/dotty that referenced this issue Apr 4, 2022
olsdavis pushed a commit to olsdavis/dotty that referenced this issue Apr 4, 2022
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Spree
Fixed
Development

Successfully merging a pull request may close this issue.

6 participants