zinc icon indicating copy to clipboard operation
zinc copied to clipboard
verified publisher icon sbt

Undercompilation when macros are used (mainargs)

Open lefou opened this issue 2 years ago • 1 comments

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

  • This reproducer uses the following versions:

    • Mill 0.10.11
    • Zinc 1.8.0
    • Scala 2.13.10
    • mainargs 0.3.0

  • The reproducer: https://github.com/lefou/mainargs-undercompilation-reproduction

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 avatar Feb 14 '23 14:02 lefou

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 avatar Feb 14 '23 14:02 lefou

For the record, here's the reverse analysis of what might be happening from the Zinc PR https://github.com/sbt/zinc/pull/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 https://github.com/sbt/zinc/pull/1316 does.

eed3si9n avatar Apr 07 '24 23:04 eed3si9n