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

Undercompilation when macros are used (mainargs) #1171

Closed
lefou opened this issue Feb 14, 2023 · 2 comments · Fixed by #1316
Closed

Undercompilation when macros are used (mainargs) #1171

lefou opened this issue Feb 14, 2023 · 2 comments · Fixed by #1316
Assignees
Labels
area/under_compilation Zinc does not pick up compilation when needed

Comments

@lefou
Copy link
Contributor

lefou commented Feb 14, 2023

Problem

When using the macro-based CLI library mainargs, I noticed an undercompilation issue

When changing the @doc annotation of one argument in foo.Config, a recompile results in only one recompiled file Config.scala.

The file ConfigParser.scala gets not recompiled, yet it contains a lazy val usageText = parser.helpText(). parser is a private[this] lazy val parser: ParserForClass[Config] = mainargs.ParserForClass[Config].

The expectation is, that also ConfigParser.scala gets recompiled. This does not happen. This results in an outdate/stale help message.

Info to reproduce

Steps to reproduce

  • Compile and run reproduction project
$ mill clean
[1/1] clean
$ mill foo.run --help
[33/46] foo.compile
Compiling compiler interface...
[info] compiling 2 Scala sources to /tmp/mainargs-repro/out/foo/compile.dest/classes ...
[info] done compiling
[46/46] foo.run
apply
  --help  Print this help message.

  • Change the contents of the @doc annotation
$ sed -i "s/Print this help message/Print another message/" foo/src/Config.scala
  • Re-run the reproduction build and notice, that only one source file gets recompiled and the --help output has not changed.
$ mill foo.run --help
[33/46] foo.compile
[info] compiling 1 Scala source to /tmp/mainargs-repro/out/foo/compile.dest/classes ... 
[info] done compiling
[46/46] foo.run
apply
  --help  Print this help message.

expectation

The compiler should also recompile the file which depends on Config.scala.

@lefou
Copy link
Contributor Author

lefou commented Feb 14, 2023

Here is the reproduction with mill --debug:

$ mill clean
[1/1] clean 
$ mill --debug foo.run --help
Using explicit system properties: Map(mill.main.cli -> /home/lefou/bin/mill)
[33/46] foo.compile 
Compiling compiler interface...
[debug] [zinc] IncrementalCompile -----------
[debug] IncrementalCompile.incrementalCompile
[debug] previous = Stamps for: 0 products, 0 sources, 0 libraries
[debug] current source = Set(/tmp/mainargs-repro/foo/src/Config.scala, /tmp/mainargs-repro/foo/src/ConfigParser.scala)
[debug] > initialChanges = InitialChanges(Changes(added = Set(/tmp/mainargs-repro/foo/src/ConfigParser.scala, /tmp/mainargs-repro/foo/src/Config.scala), removed = Set(), changed = Set(), unmodified = ...),Set(),Set(),API Changes: Set())
[debug] Full compilation, no sources in previous analysis.
[debug] all 2 sources are invalidated
[debug] Initial set of included nodes: 
[debug] Recompiling all sources: number of invalidated sources > 50.0 percent of all sources
[debug] compilation cycle 1
[info] compiling 2 Scala sources to /tmp/mainargs-repro/out/foo/compile.dest/classes ...
[debug] Returning already retrieved and compiled bridge: /tmp/mainargs-repro/out/mill/scalalib/ZincWorkerModule/worker.dest/zinc-1.8.0/2.13.10/compiled.
[debug] [zinc] Running cached compiler 66d9c507 for Scala compiler version 2.13.10
[debug] [zinc] The Scala compiler is invoked with:
[debug]         -bootclasspath
[debug]         /home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar
[debug]         -classpath
[debug]         /tmp/mainargs-repro/foo/resources:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/mainargs_2.13/0.3.0/mainargs_2.13-0.3.0.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_2.13/2.8.1/scala-collection-compat_2.13-2.8.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/pprint_2.13/0.7.3/pprint_2.13-0.7.3.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/fansi_2.13/0.3.1/fansi_2.13-0.3.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/sourcecode_2.13/0.2.8/sourcecode_2.13-0.2.8.jar:/tmp/mainargs-repro/out/foo/compile.dest/classes
[debug] Scala compilation took 3.232703598 s
[info] done compiling
[46/46] foo.run 
Run subprocess with args: '/opt/openjdk-bin-11.0.18_p10/bin/java' '-cp' '/tmp/mainargs-repro/foo/resources:/tmp/mainargs-repro/out/foo/compile.dest/classes:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/mainargs_2.13/0.3.0/mainargs_2.13-0.3.0.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_2.13/2.8.1/scala-collection-compat_2.13-2.8.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/pprint_2.13/0.7.3/pprint_2.13-0.7.3.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/fansi_2.13/0.3.1/fansi_2.13-0.3.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/sourcecode_2.13/0.2.8/sourcecode_2.13-0.2.8.jar' 'foo.ConfigParser' '--help'
apply
  --help  Print this help message.

$ sed -i "s/Print this help message/Print another message/" foo/src/Config.scala 
$ mill --debug foo.run --help
Using explicit system properties: Map(mill.main.cli -> /home/lefou/bin/mill)
[33/46] foo.compile 
[debug] [zinc] IncrementalCompile -----------
[debug] IncrementalCompile.incrementalCompile
[debug] previous = Stamps for: 4 products, 2 sources, 2 libraries
[debug] current source = Set(/tmp/mainargs-repro/foo/src/Config.scala, /tmp/mainargs-repro/foo/src/ConfigParser.scala)
[debug] > initialChanges = InitialChanges(Changes(added = Set(), removed = Set(), changed = Set(/tmp/mainargs-repro/foo/src/Config.scala), unmodified = ...),Set(),Set(),API Changes: Set())
[debug] 
[debug] Initial source changes:
[debug]         removed: Set()
[debug]         added: Set()
[debug]         modified: Set(/tmp/mainargs-repro/foo/src/Config.scala)
[debug] Invalidated products: Set()
[debug] External API changes: API Changes: Set()
[debug] Modified binary dependencies: Set()
[debug] Initial directly invalidated classes: Set(foo.Config)
[debug] Sources indirectly invalidated by:
[debug]         product: Set()
[debug]         binary dep: Set()
[debug]         external source: Set()
[debug] All initially invalidated classes: Set(foo.Config)
[debug] All initially invalidated sources:Set(/tmp/mainargs-repro/foo/src/Config.scala)
[debug] Initial set of included nodes: foo.Config
[debug] compilation cycle 1
[info] compiling 1 Scala source to /tmp/mainargs-repro/out/foo/compile.dest/classes ...
[debug] Returning already retrieved and compiled bridge: /tmp/mainargs-repro/out/mill/scalalib/ZincWorkerModule/worker.dest/zinc-1.8.0/2.13.10/compiled.
[debug] [zinc] Running cached compiler 3341fd40 for Scala compiler version 2.13.10
[debug] [zinc] The Scala compiler is invoked with:
[debug]         -bootclasspath
[debug]         /home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar
[debug]         -classpath
[debug]         /tmp/mainargs-repro/foo/resources:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/mainargs_2.13/0.3.0/mainargs_2.13-0.3.0.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_2.13/2.8.1/scala-collection-compat_2.13-2.8.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/pprint_2.13/0.7.3/pprint_2.13-0.7.3.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/fansi_2.13/0.3.1/fansi_2.13-0.3.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/sourcecode_2.13/0.2.8/sourcecode_2.13-0.2.8.jar:/tmp/mainargs-repro/out/foo/compile.dest/classes
[debug] New invalidations:
[debug] Initial set of included nodes: 
[debug] Previously invalidated, but (transitively) depend on new invalidations:
[debug] Final step, transitive dependencies:
[debug]         Set()
[debug] No classes were invalidated.
[debug] Scala compilation took 0.411515453 s
[info] done compiling
[46/46] foo.run 
Run subprocess with args: '/opt/openjdk-bin-11.0.18_p10/bin/java' '-cp' '/tmp/mainargs-repro/foo/resources:/tmp/mainargs-repro/out/foo/compile.dest/classes:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/mainargs_2.13/0.3.0/mainargs_2.13-0.3.0.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.10/scala-library-2.13.10.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-collection-compat_2.13/2.8.1/scala-collection-compat_2.13-2.8.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/pprint_2.13/0.7.3/pprint_2.13-0.7.3.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/fansi_2.13/0.3.1/fansi_2.13-0.3.1.jar:/home/lefou/.cache/coursier/v1/https/repo1.maven.org/maven2/com/lihaoyi/sourcecode_2.13/0.2.8/sourcecode_2.13-0.2.8.jar' 'foo.ConfigParser' '--help'
apply
  --help  Print this help message.


@lefou lefou changed the title Undercompilation of macro-code (mainargs) Undercompilation when macros are used (mainargs) Feb 14, 2023
@eed3si9n eed3si9n added the area/under_compilation Zinc does not pick up compilation when needed label Feb 14, 2023
Friendseeker added a commit to Friendseeker/zinc that referenced this issue Nov 22, 2023
eed3si9n added a commit that referenced this issue Nov 24, 2023
@Friendseeker Friendseeker self-assigned this Dec 19, 2023
@eed3si9n
Copy link
Member

eed3si9n commented Apr 7, 2024

For the record, here's the reverse analysis of what might be happening from the Zinc PR #1316 by @Friendseeker. At https://github.com/lefou/mainargs-undercompilation-reproduction/blob/f1ee0417cab74d721063668ea5247f044870f958/foo/src/ConfigParser.scala#L7C3-L7C90

  private[this] lazy val parser: ParserForClass[Config] = mainargs.ParserForClass[Config]

as reported by OP.

https://github.com/com-lihaoyi/mainargs/blob/e1c8e25b93d1f652465dabd308aa268cb6e23696/mainargs/src/Parser.scala#L264

object ParserForClass extends ParserForClassCompanionVersionSpecific

and for Scala 2, ParserForClassCompanionVersionSpecific would be:

private[mainargs] trait ParserForClassCompanionVersionSpecific {
  def apply[T]: ParserForClass[T] = macro Macros.parserForClass[T]
}

Macros.parserForClass[T]

The macro code looks like this https://github.com/com-lihaoyi/mainargs/blob/e1c8e25b93d1f652465dabd308aa268cb6e23696/mainargs/src-2/Macros.scala#L24-L44

  def parserForClass[T: c.WeakTypeTag]: c.Tree = {

    val cls = weakTypeOf[T].typeSymbol.asClass
    val companionObj = weakTypeOf[T].typeSymbol.companion
    val constructor = cls.primaryConstructor.asMethod
    val route = extractMethod(
      TermName("apply"),
      constructor.paramLists.flatten,
      constructor.pos,
      cls.annotations.find(_.tpe =:= typeOf[main]),
      companionObj.typeSignature,
      weakTypeOf[T]
    )

    q"""
      new _root_.mainargs.ParserForClass(
        $route.asInstanceOf[_root_.mainargs.MainData[${weakTypeOf[T]}, Any]],
        () => $companionObj
      )
    """
  }

so in here, the ParserForClass.apply[Config] macro reads the parameter list of Config, and supposedly that's where it reads the annotation for the parameter list.

If we treat ParserForClass.apply[Config] as a simple function call, Zinc's current behavior of not invalidating the parser makes sense since the change of annotation does not change the binary API of the Config class, and Zinc invalidates the caller only if the signature change affects the inheritance of method calls.

expanding the scope of invalidation

If we treat ParserForClass.apply[Config] as an meta-programmed tree, even without knowing anything about its implementation, it does make sense to invalidate the entire compilation unit containing the macro application because its input (in this case Config) changed.

I think that's what #1316 does.

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

Successfully merging a pull request may close this issue.

3 participants