Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 15 additions & 5 deletions compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package sbt
import java.io.File
import java.nio.file.Path
import java.util.EnumSet

import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.classpath.FileUtils.{hasClassExtension, hasTastyExtension}
import dotty.tools.dotc.core.Contexts.*
Expand All @@ -16,16 +15,15 @@ import dotty.tools.dotc.core.Phases.*
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.core.Denotations.StaleSymbol
import dotty.tools.dotc.core.Types.*

import dotty.tools.dotc.util.{SrcPos, NoSourcePosition}
import dotty.tools.dotc.typer.Applications.*
import dotty.tools.dotc.util.{NoSourcePosition, SrcPos}
import dotty.tools.io
import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive, NoAbstractFile, FileExtension}
import dotty.tools.io.{AbstractFile, FileExtension, NoAbstractFile, PlainFile, ZipArchive}
import xsbti.UseScope
import xsbti.api.DependencyContext
import xsbti.api.DependencyContext.*

import scala.jdk.CollectionConverters.*

import scala.collection.{Set, mutable}
import scala.compiletime.uninitialized

Expand Down Expand Up @@ -265,6 +263,18 @@ trait AbstractExtractDependenciesCollector(rec: DependencyRecorder) extends tpd.
addInheritanceDependencies(t)
case t: Template =>
addInheritanceDependencies(t)
case UnApply(fun, implicits, patterns) =>
// For a case-class unapply of the form `def unapply(x: C): C = x`, the
// compiler emits calls to the product selector methods `_1`, `_2`, … in
// the generated bytecode (via PatternMatcher, which runs after this phase).
// Those selectors are never explicit in the typed tree, so they would not
// normally be recorded as used names. We record them here so that Zinc
// knows to recompile this file if the return type of any selector changes.
//
// fun.tpe is a TermRef; widen first to reach the underlying MethodType,
// then take finalResultType to get the case class type (e.g. Customer2).
val selectors = productSelectors(fun.tpe.widen.finalResultType)
selectors.foreach(addMemberRefDependency)
case _ => ()

/**Reused EqHashSet, safe to use as each TypeDependencyTraverser is used atomically
Expand Down
94 changes: 94 additions & 0 deletions sbt-bridge/test/xsbt/ExtractAPISpecification.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package xsbt

import xsbti.UseScope
import xsbti.api._
import xsbt.api.SameAPI

Expand Down Expand Up @@ -168,6 +169,99 @@ class ExtractAPISpecification {
assertNotEquals(apis("A.AA"), apis("B.AA"))
}

// Tests for https://github.com/scala/scala3/issues/26231
// Undercompilation when a case class field type changes and another file pattern-matches on it.

/** Returns the declared members of the API for a named class/module in a given source. */
private def declaredMembers(src: String, className: String, defType: DefinitionType): Array[ClassDefinition] = {
val apis = new ScalaCompilerForUnitTesting().extractApiFromSrc(src)
val cls = apis.find(c => c.name() == className && c.definitionType() == defType).get
cls.structure().declared()
}

@Test
def caseClassSelectorMethodApiChangesWhenFieldTypeChanges = {
// `_1` is the product selector called at the bytecode level after pattern matching.
// Its return type should change when the case class field type changes.
val withNumber = declaredMembers(
"case class Customer2(state: Number = java.lang.Integer.valueOf(1))",
"Customer2", DefinitionType.ClassDef
)
val withString = declaredMembers(
"""case class Customer2(state: String = "")""",
"Customer2", DefinitionType.ClassDef
)

val selector1WithNumber = withNumber.find(_.name() == "_1").get
val selector1WithString = withString.find(_.name() == "_1").get

assertFalse(
"_1 API must differ between Number and String field type",
SameAPI(selector1WithNumber, selector1WithString)
)
}

@Test
def companionUnapplyApiUnchangedWhenCaseClassFieldTypeChanges = {
// In Scala 3, the compiler generates `def unapply(x: Customer2): Customer2 = x` for
// case classes. Its signature is always `(Customer2): Customer2` regardless of field
// types, so its API hash never changes when a field type changes.
//
// This is one half of the undercompilation bug: the dependent file (Test.scala) records
// a used-name dependency on `unapply`, but `unapply`'s hash doesn't change, so Zinc
// doesn't recompile Test.scala even though _1's return type changed.
val unapplyWithNumber = declaredMembers(
"case class Customer2(state: Number = java.lang.Integer.valueOf(1))",
"Customer2", DefinitionType.Module
).find(_.name() == "unapply").get

val unapplyWithString = declaredMembers(
"""case class Customer2(state: String = "")""",
"Customer2", DefinitionType.Module
).find(_.name() == "unapply").get

assertTrue(
"unapply API is unchanged between Number and String field types (the stable signature that hides the change from Zinc)",
SameAPI(unapplyWithNumber, unapplyWithString)
)
}

@Test
def caseClassPatternMatchRecordsSelector1AsUsedName = {
// The other half of the undercompilation bug: when Test.scala does
// case Customer2(x) => x
// the compiler emits a call to Customer2._1() in the bytecode. But
// ExtractDependencies only records `unapply` as a used name (from the
// UnApply tree node), never `_1`. Since `unapply`'s hash doesn't change
// when field types change, Zinc won't recompile Test.scala.
//
// This test asserts the DESIRED (fixed) behaviour: `_1` must appear in the
// used names so that a change in _1's return type triggers recompilation.
// It is expected to FAIL on an unfixed compiler, documenting the bug.
val caseClassSrc =
"case class Customer2(state: Number = java.lang.Integer.valueOf(1))"
val patternMatchSrc =
"""|object Test {
| def test(c: Option[Customer2]): Any = c match {
| case Some(Customer2(x)) => x
| case None => null
| }
|}""".stripMargin

// compileSrcs returns used names for all classes across all source files
val output = new ScalaCompilerForUnitTesting()
.compileSrcs(caseClassSrc, patternMatchSrc)
val usedNames = output.analysis.usedNames

// `unapply` is recorded because the typed UnApply tree references Customer2.unapply.
assertTrue("unapply must be a used name in Test",
usedNames("Test").contains("unapply"))

// `_1` must also be recorded because it is what the generated bytecode calls.
assertTrue("_1 must be a used name in Test",
usedNames("Test").contains("_1"))
}

@Test
def handlePackageObjectsAndTypeCompanions = {
val src =
Expand Down
Loading