Support zinc invalidation for type arguments in macro calls #23900
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @bishabosha, would you be able to review this? If not, then I think I can ask @tgodzik, but he's more of an expert on the zinc side than the compiler side I think. |
lrytz
approved these changes
Sep 18, 2025
| private def addMacroDependency(sym: Symbol)(using Context): Unit = | ||
| if (allowsDependencyByMacroExpansion) { | ||
| if (!ignoreDependency(sym)) { | ||
| if (!sym.is(Package)) { |
There was a problem hiding this comment.
Isn't the package test redundant, because it's already in TypeDependencyTraverser.traverse?
bishabosha
reviewed
Sep 18, 2025
| val traverser = new TypeDependencyTraverser { | ||
| def addDependency(symbol: Symbol) = addMacroDependency(symbol) | ||
| } | ||
| trees.foreach(tree => traverser.traverse(tree.tpe)) |
There was a problem hiding this comment.
Looks like this whole function should be elided when the macro dependency tracking is unavailable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #23852
Closes #19426
Around a year ago a new
DependencyContextvalue was added to the zinc api:DependencyByMacroExpansion.It causes the recorded dependency to trigger recompilation if there is any API change in the recorded type, not just if the feudal/method member that was explicitly referenced was changed. So now, for every inline call:
macroCall[T1, T2, ...], in filecall.scala, if anything changes in a TN definition,call.scalais recompiled.Since
DependencyByMacroExpansionwas added later to the API, referencing it in older versions (e.g. any sbt <1.10.0), would cause crashes. to avoid that, we use reflection to check if that field exists.These changes do not seem to affect #23783, likely the way annotations are handled needs to be changed in zinc itself, as the APIInfo phase does successfully record the change in the annotation argument.
Related discussion: sbt/zinc#1316 (a PR adding the functionality to zinc and scala-2 plugin).