Skip to content

Cyclic reference when using List and compiling Seq* classes at the same time #970

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
Closed
DarkDimius opened this issue Nov 23, 2015 · 6 comments
Closed

Cyclic reference when using List and compiling Seq* classes at the same time #970

DarkDimius opened this issue Nov 23, 2015 · 6 comments
Labels
area:typer itype:bug

Comments

@DarkDimius
Copy link
Contributor 

$ cat B.scala
object B{
  def main(args: Array[String]): Unit = {
    val s = List(1,2,3)
    ()
  }
}

$ dotc ./scala-scala/src/library/scala/collection/immutable/Seq.scala ./scala-scala/src/library/scala/package.scala ./scala-scala/src/library/scala/collection/GenSeqLike.scala ./scala-scala/src/library/scala/collection/SeqLike.scala ./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala B.scala
./scala-scala/src/library/scala/collection/SeqLike.scala:62: error: cyclic reference involving trait ParSeq
trait SeqLike[+A, +Repr] extends Any with IterableLike[A, Repr] with GenSeqLike[A, Repr] with Parallelizable[A, ParSeq[A]] { self =>
                                                                                                                ^

If B.scala is removed from compilation list, cyclic reference error is not triggered anymore.
@smarter
Copy link
Member

smarter commented Nov 24, 2015

Can you still reproduce this? Seems to work fine here now.

@DarkDimius
Copy link
Contributor Author

I can reproduce it on 4463f5e locally.

@DarkDimius
Copy link
Contributor Author

@smarter could you please try with different order of arguments:

dotc B.scala ./scala-scala/src/library/scala/collection/immutable/Seq.scala ./scala-scala/src/library/scala/package.scala ./scala-scala/src/library/scala/collection/GenSeqLike.scala ./scala-scala/src/library/scala/collection/SeqLike.scala ./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala

@smarter
Copy link
Member

smarter commented Nov 25, 2015

Yes, I can reproduce that way.

smarter added a commit to dotty-staging/dotty that referenced this issue Nov 27, 2015
@smarter
[PROOF-OF-CONCEPT] Avoid forcing type aliases to get their typeParams
83d43d3 
This is a strawman proposal for fixing scala#970, before this commit, the
following fails with a cyclic reference error:

dotc B.scala\
./scala-scala/src/library/scala/collection/immutable/Seq.scala\
./scala-scala/src/library/scala/package.scala\
./scala-scala/src/library/scala/collection/GenSeqLike.scala\
./scala-scala/src/library/scala/collection/SeqLike.scala\
./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala\

where B.scala is defined as:
object B{
  def main(args: Array[String]): Unit = {
    val s = List(1,2,3)
    ()
  }
}

Here's what I think is happening:
1. Unpickling a Scala2 TypeRef requires us to perform type application.
   (`Scala2Unpickler#readType`)
2. To apply a TypeAlias to some arguments, we first need to determine its
   type parameters.
   (`TypeApplications#appliedTo` and `TypeApplications#typeParams`)
3. To do this, we look at the type parameters of the underlying type of
   the TypeAlias, this requires completing the TypeAlias.
4. If the TypeAlias is defined in a source tree and not unpickled, this
   forces us to typecheck its right-hand side.
   (`Namer#Completer#typeSig`)
5. In turns, this forces various classes to be completed, which might
   themselves refer indirectly to type aliases, forcing even more
   stuff.

This commit is a hacky way to avoid 3. by only completing the type parameters
of a type alias instead of completing the whole type alias when we don't
need it. Let me know if you think the basic idea is sound or not and if
you can think of a nicer way to implement it!
smarter added a commit to dotty-staging/dotty that referenced this issue Nov 27, 2015
@smarter
[PROOF-OF-CONCEPT] Avoid forcing type aliases to get their typeParams
c4ea1e7 
This is a strawman proposal for fixing scala#970, before this commit, the
following fails with a cyclic reference error:

dotc B.scala\
./scala-scala/src/library/scala/collection/immutable/Seq.scala\
./scala-scala/src/library/scala/package.scala\
./scala-scala/src/library/scala/collection/GenSeqLike.scala\
./scala-scala/src/library/scala/collection/SeqLike.scala\
./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala

where B.scala is defined as:
object B{
  def main(args: Array[String]): Unit = {
    val s = List(1,2,3)
    ()
  }
}

Here's what I think is happening:
1. Unpickling a Scala2 TypeRef requires us to perform type application.
   (`Scala2Unpickler#readType`)
2. To apply a TypeAlias to some arguments, we first need to determine its
   type parameters.
   (`TypeApplications#appliedTo` and `TypeApplications#typeParams`)
3. To do this, we look at the type parameters of the underlying type of
   the TypeAlias, this requires completing the TypeAlias.
4. If the TypeAlias is defined in a source tree and not unpickled, this
   forces us to typecheck its right-hand side.
   (`Namer#Completer#typeSig`)
5. In turns, this forces various classes to be completed, which might
   themselves refer indirectly to type aliases, forcing even more
   stuff.

This commit is a hacky way to avoid 3. by only completing the type parameters
of a type alias instead of completing the whole type alias when we don't
need it. Let me know if you think the basic idea is sound or not and if
you can think of a nicer way to implement it!
@smarter smarter mentioned this issue Nov 27, 2015
Closed
@smarter
Copy link
Member

smarter commented Nov 27, 2015

OK, I think I got it but I'll need some help with the design: #981

smarter added a commit to dotty-staging/dotty that referenced this issue Nov 30, 2015
@smarter
[PROOF-OF-CONCEPT] Avoid forcing type aliases to get their typeParams
69ed9b7 
This is a strawman proposal for fixing scala#970, before this commit, the
following fails with a cyclic reference error:

dotc B.scala\
./scala-scala/src/library/scala/collection/immutable/Seq.scala\
./scala-scala/src/library/scala/package.scala\
./scala-scala/src/library/scala/collection/GenSeqLike.scala\
./scala-scala/src/library/scala/collection/SeqLike.scala\
./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala

where B.scala is defined as:
object B{
  def main(args: Array[String]): Unit = {
    val s = List(1,2,3)
    ()
  }
}

Here's what I think is happening:
1. Unpickling a Scala2 TypeRef requires us to perform type application.
   (`Scala2Unpickler#readType`)
2. To apply a TypeAlias to some arguments, we first need to determine its
   type parameters.
   (`TypeApplications#appliedTo` and `TypeApplications#typeParams`)
3. To do this, we look at the type parameters of the underlying type of
   the TypeAlias, this requires completing the TypeAlias.
4. If the TypeAlias is defined in a source tree and not unpickled, this
   forces us to typecheck its right-hand side.
   (`Namer#Completer#typeSig`)
5. In turns, this forces various classes to be completed, which might
   themselves refer indirectly to type aliases, forcing even more
   stuff.

This commit is a hacky way to avoid 3. by only completing the type parameters
of a type alias instead of completing the whole type alias when we don't
need it. Let me know if you think the basic idea is sound or not and if
you can think of a nicer way to implement it!
This was referenced Jan 15, 2016
Closed
Closed
@odersky
Copy link
Contributor

odersky commented Feb 12, 2016

This is fixed by now.

@odersky odersky closed this as completed Feb 12, 2016
DarkDimius pushed a commit to dotty-linker/dotty that referenced this issue May 9, 2016
@smarter @DarkDimius
[PROOF-OF-CONCEPT] Avoid forcing type aliases to get their typeParams
d2757f2 
This is a strawman proposal for fixing scala#970, before this commit, the
following fails with a cyclic reference error:

dotc B.scala\
./scala-scala/src/library/scala/collection/immutable/Seq.scala\
./scala-scala/src/library/scala/package.scala\
./scala-scala/src/library/scala/collection/GenSeqLike.scala\
./scala-scala/src/library/scala/collection/SeqLike.scala\
./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala

where B.scala is defined as:
object B{
  def main(args: Array[String]): Unit = {
    val s = List(1,2,3)
    ()
  }
}

Here's what I think is happening:
1. Unpickling a Scala2 TypeRef requires us to perform type application.
   (`Scala2Unpickler#readType`)
2. To apply a TypeAlias to some arguments, we first need to determine its
   type parameters.
   (`TypeApplications#appliedTo` and `TypeApplications#typeParams`)
3. To do this, we look at the type parameters of the underlying type of
   the TypeAlias, this requires completing the TypeAlias.
4. If the TypeAlias is defined in a source tree and not unpickled, this
   forces us to typecheck its right-hand side.
   (`Namer#Completer#typeSig`)
5. In turns, this forces various classes to be completed, which might
   themselves refer indirectly to type aliases, forcing even more
   stuff.

This commit is a hacky way to avoid 3. by only completing the type parameters
of a type alias instead of completing the whole type alias when we don't
need it. Let me know if you think the basic idea is sound or not and if
you can think of a nicer way to implement it!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area:typer itype:bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@smarter @odersky @DarkDimius