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.

Sign up for GitHub

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

Jump to bottom

Ensure macro changes invalidate downstream classes #1496

Closed
wants to merge 1 commit into from
Closed

Ensure macro changes invalidate downstream classes #1496

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions ...al/zinc-persist/src/main/scala/sbt/internal/inc/consistent/ConsistentAnalysisFormat.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,7 @@ class ConsistentAnalysisFormat(val mappers: ReadWriteMappers, sort: Boolean) {

private[this] def writeAPIs(out: Serializer, apis: APIs, storeApis: Boolean): Unit = {
def write(n: String, m: Map[String, AnalyzedClass]): Unit =
writeMaybeSortedStringMap(
out,
n,
m.mapValues(_.withCompilationTimestamp(DefaultCompilationTimestamp))
) { ac =>
writeMaybeSortedStringMap(out, n, m) { ac =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would effectively make the analysis format not consistent, so I don't think this is a good direction. I thought by default Zinc would invalidate any macro calls? /cc @szeiger

Copy link
Member

@Friendseeker Friendseeker Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default Zinc would invalidate a macro call iff the compilation timestamp in associated AnalyzedClass before & after mismatches.

zinc/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala

Lines 310 to 326 in ced4262

def classDiff(className: String, a: AnalyzedClass, b: AnalyzedClass): Option[APIChange] = {
// log.debug(s"[zinc] classDiff($className, ${a.name}, ${b.name})")
if (a.compilationTimestamp() == b.compilationTimestamp() && (a.apiHash == b.apiHash)) None
else {
val hasMacro = a.hasMacro || b.hasMacro
if (hasMacro && IncOptions.getRecompileOnMacroDef(options)) {
Some(APIChangeDueToMacroDefinition(className))
} else if (
APIUtil.isAnnotationDefinition(a.api().classApi()) ||
APIUtil.isAnnotationDefinition(b.api().classApi())
) {
Some(APIChangeDueToAnnotationDefinition(className))
} else {
findAPIChange(className, a, b)
}
}
}

Hence this PR does point out a real undercompilation issue but our discussed solution of using protobuf as default 1.x format and using product hash based invalidation (to replace timestamp based invalidation) in 2.x branch should effectively address the issue.

writeAnalyzedClass(out, ac, storeApis)
}
write("internal", apis.internal)
Expand Down
Loading