Skip to content

Commit

Permalink
Merge pull request #10626 from lrytz/t12814-retro
Browse files Browse the repository at this point in the history
Report fewer spurious cyclic errors
  • Loading branch information
lrytz committed Dec 13, 2023
2 parents 459625f + 5928d4c commit 43aa816
Show file tree
Hide file tree
Showing 9 changed files with 335 additions and 39 deletions.
4 changes: 1 addition & 3 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Expand Up @@ -919,12 +919,10 @@ trait ContextErrors extends splain.SplainErrors {
}

// cyclic errors

def CyclicAliasingOrSubtypingError(errPos: Position, sym0: Symbol) =
issueTypeError(PosAndMsgTypeError(errPos, "cyclic aliasing or subtyping involving "+sym0))

def CyclicReferenceError(errPos: Position, tp: Type, lockedSym: Symbol) =
issueTypeError(PosAndMsgTypeError(errPos, s"illegal cyclic reference involving $tp and $lockedSym"))

// macro-related errors (also see MacroErrors below)

def MacroEtaError(tree: Tree) = {
Expand Down
12 changes: 0 additions & 12 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Expand Up @@ -863,12 +863,6 @@ trait Namers extends MethodSynthesis {
def monoTypeCompleter(tree: MemberDef) = new MonoTypeCompleter(tree)
class MonoTypeCompleter(tree: MemberDef) extends TypeCompleterBase(tree) {
override def completeImpl(sym: Symbol): Unit = {
// this early test is there to avoid infinite baseTypes when
// adding setters and getters --> bug798
// It is a def in an attempt to provide some insulation against
// uninitialized symbols misleading us. It is not a certainty
// this accomplishes anything, but performance is a non-consideration
// on these flag checks so it can't hurt.
def needsCycleCheck = sym.isNonClassType && !sym.isParameter && !sym.isExistential

val annotations = annotSig(tree.mods.annotations, tree, _ => true)
Expand All @@ -887,12 +881,6 @@ trait Namers extends MethodSynthesis {

sym.setInfo(if (!sym.isJavaDefined) tp else RestrictJavaArraysMap(tp))

if (needsCycleCheck) {
log(s"Needs cycle check: ${sym.debugLocationString}")
if (!typer.checkNonCyclic(tree.pos, tp))
sym setInfo ErrorType
}

validate(sym)
}
}
Expand Down
53 changes: 33 additions & 20 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -401,43 +401,56 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
)
}

/** Check that type `tp` is not a subtype of itself.
*/
def checkNonCyclic(pos: Position, tp: Type): Boolean = {
def checkNotLocked(sym: Symbol) = {
sym.initialize.lockOK || { CyclicAliasingOrSubtypingError(pos, sym); false }
class NonCyclicStack {
// for diverging types, neg/t510.scala
private val maxRecursion = 42

// For each abstract type symbol (type member, type parameter), keep track of seen types represented by that symbol
private lazy val map = collection.mutable.HashMap[Symbol, mutable.ListBuffer[Type]]()

def lockSymbol[T](sym: Symbol, tp: Type)(body: => T): T = {
val stk = map.getOrElseUpdate(sym, ListBuffer.empty)
stk.prepend(tp)
try body
finally stk.remove(0)
}

def isUnlocked(sym: Symbol, tp: Type): Boolean =
!sym.isNonClassType || !map.get(sym).exists(tps => tps.length > maxRecursion || tps.contains(tp))
}

/** Check that type `tp` is not a subtype of itself
*/
def checkNonCyclic(pos: Position, tp: Type, stack: NonCyclicStack = new NonCyclicStack): Boolean = {
def checkNotLocked(sym: Symbol) =
stack.isUnlocked(sym, tp) || { CyclicAliasingOrSubtypingError(pos, sym); false }

tp match {
case TypeRef(pre, sym, args) =>
checkNotLocked(sym) &&
((!sym.isNonClassType) || checkNonCyclic(pos, appliedType(pre.memberInfo(sym), args), sym))
// @M! info for a type ref to a type parameter now returns a polytype
// @M was: checkNonCyclic(pos, pre.memberInfo(sym).subst(sym.typeParams, args), sym)
checkNotLocked(sym) && {
!sym.isNonClassType ||
stack.lockSymbol(sym, tp) {
checkNonCyclic(pos, appliedType(pre.memberInfo(sym), args), stack)
}
}

case SingleType(_, sym) =>
checkNotLocked(sym)
case st: SubType =>
checkNonCyclic(pos, st.supertype)
checkNonCyclic(pos, st.supertype, stack)
case ct: CompoundType =>
ct.parents forall (x => checkNonCyclic(pos, x))
ct.parents forall (x => checkNonCyclic(pos, x, stack))
case _ =>
true
}
}

def checkNonCyclic(pos: Position, tp: Type, lockedSym: Symbol): Boolean = try {
if (!lockedSym.lock(CyclicReferenceError(pos, tp, lockedSym))) false
else checkNonCyclic(pos, tp)
} finally {
lockedSym.unlock()
}

def checkNonCyclic(sym: Symbol): Unit = {
if (!checkNonCyclic(sym.pos, sym.tpe_*)) sym.setInfo(ErrorType)
}

def checkNonCyclic(defn: Tree, tpt: Tree): Unit = {
if (!checkNonCyclic(defn.pos, tpt.tpe, defn.symbol)) {
def checkNonCyclic(defn: ValOrDefDef, tpt: Tree): Unit = {
if (!checkNonCyclic(defn.pos, tpt.tpe)) {
tpt setType ErrorType
defn.symbol.setInfo(ErrorType)
}
Expand Down
10 changes: 10 additions & 0 deletions test/files/pos/t10669.scala
@@ -0,0 +1,10 @@
abstract class CharSet[P] {
type Type <: P
}

object LetterOrDigit extends CharSet[Char]
object Digit extends CharSet[LetterOrDigit.Type]

object t {
type D = Digit.Type
}
43 changes: 43 additions & 0 deletions test/files/pos/t12622.scala
@@ -0,0 +1,43 @@
trait ScenarioParam {
type Builder <: Type
}

trait ScenarioParamBuilder

trait Type {
type Builder <: ScenarioParamBuilder
}

trait Types[H <: ScenarioParam, T <: Type] extends Type {
type Builder = H#Builder with T#Builder
}

trait Nil extends Type {
type Builder = ScenarioParamBuilder
}

trait ScenarioTarget {
type FilterParam <: Type
}

class P1 extends ScenarioParam
class P2 extends ScenarioParam

object someTarget extends ScenarioTarget {
type FilterParam = Types[P1, Types[P2, Nil]]
}

class WhereClauseBuilder1[T <: ScenarioTarget] {
type FilterBuilderType = T#FilterParam#Builder
def m1(f: FilterBuilderType => Any): Any = null
def m2(f: T#FilterParam#Builder => Any): Any = null
}

object t {
(null: WhereClauseBuilder1[someTarget.type]).m1(x => null)

val stabilizer: WhereClauseBuilder1[someTarget.type] = null
stabilizer.m1(x => null)

(null: WhereClauseBuilder1[someTarget.type]).m2(x => null)
}
98 changes: 98 additions & 0 deletions test/files/pos/t12814.scala
@@ -0,0 +1,98 @@
// https://github.com/scala/bug/issues/12814#issuecomment-1822770100
object t1 {
trait A[X] { type T = X }
object B extends A[String]
object C extends A[B.T] {
def f: C.T = "hai"
}
}

// https://github.com/scala/bug/issues/12814
object t2 {
sealed trait Common
sealed trait One extends Common
sealed trait Two extends Common


trait Module[C <: Common] {
val name: String
type Narrow = C
def narrow: PartialFunction[Common, C]
}

object ModuleA extends Module[One] {
val name = "A"
val narrow: PartialFunction[Common, Narrow] = {
case cc: Narrow => cc
}
}

object ModuleB extends Module[ModuleA.Narrow] {
val name = "B"
val narrow: PartialFunction[Common, Narrow] = {
case cc: Narrow => cc
}
}

object ModuleC extends Module[Two] {
val name = "C"
val narrow: PartialFunction[Common, Narrow] = {
case cc: Narrow => cc
}
}

object ModuleD extends Module[One with Two] {
val name = "D"
val narrow: PartialFunction[Common, Narrow] = {
case cc: Narrow => cc
}
}

val one = new One {}
val two = new Two {}
val oneTwo = new One with Two {}

Seq(ModuleA, ModuleB, ModuleC, ModuleD).foreach { module =>
println(s"${module.name} at One = ${module.narrow.isDefinedAt(one)}")
println(s"${module.name} at Two = ${module.narrow.isDefinedAt(two)}")
println(s"${module.name} at OneTwo = ${module.narrow.isDefinedAt(oneTwo)}")
println("-" * 10)
}
}

// https://github.com/scala/scala/pull/10457/files
object t3 {
sealed trait A

sealed trait B extends A

trait F[C] {
type T = C
}

object O extends F[B]

object P1 extends F[O.T] {
val f: PartialFunction[A, P1.T] = {
case x: P1.T => x
}
}

object P2 extends F[O.T] {
val f: PartialFunction[A, P2.T] = x => x match {
case x: P2.T => x
}
}

object P3 extends F[O.T] {
val f: Function1[A, P3.T] = {
case x: P3.T => x
}
}

object P4 extends F[O.T] {
val f: Function1[A, P4.T] = x => x match {
case x: P4.T => x
}
}
}

0 comments on commit 43aa816

Please sign in to comment.