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.

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

Report fewer spurious cyclic errors #10626

Merged
merged 2 commits into from
Dec 13, 2023
Merged
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
4 changes: 1 addition & 3 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Copy link
Member Author

Choose a reason for hiding this comment

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

lock always succeeded. This overload of checkNonCyclic is invoked right after checkNotLocked.

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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
}
}
}
Loading