-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Support zinc invalidation for type arguments in macro calls #23900
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
Support zinc invalidation for type arguments in macro calls #23900
Conversation
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A valuable improvement!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the package test redundant, because it's already in TypeDependencyTraverser.traverse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this whole function should be elided when the macro dependency tracking is unavailable
Uh oh!
There was an error while loading. Please reload this page.
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).