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

"extends AnyRef" should not induce cyclic reference errors #6535

Closed
scabug opened this issue Oct 17, 2012 · 9 comments
Closed

"extends AnyRef" should not induce cyclic reference errors #6535

scabug opened this issue Oct 17, 2012 · 9 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Oct 17, 2012

30 errors is a lot of errors for something which should be a no-op. It's supposed to mean "extends AnyRef" already!

# 2.10.0-RC1, compiles
% scalac src/library/scala/collection/convert/Wrap*.scala 
# add 'extends AnyRef' to WrapAsScala
% git diff
-trait WrapAsScala extends LowPriorityWrapAsScala {
+trait WrapAsScala extends AnyRef with LowPriorityWrapAsScala {

% scalac src/library/scala/collection/convert/Wrap*.scala
src/library/scala/collection/convert/WrapAsScala.scala:13: error: illegal cyclic reference involving object Wrappers
import Wrappers._
       ^
[snip]
30 errors found.
@scabug
Copy link
Author

@scabug scabug commented Oct 17, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6535?orig=1
Reporter: @paulp
Affected Versions: 2.10.0-RC3

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 3, 2012

@retronym said (edited on Dec 3, 2012 11:42:07 PM UTC):
Minimization: retronym/scala@scala:2.10.x...retronym:ticket/6535

object As {
  import Bs.B._

  object A
  extends scala.Immutable // needed for the cycle; scala.{Serializable, Clonable} also work.
                          // replacing with a locally defined closs doesn't cycle.
}

object Bs {
  import As.A._

  object B
  extends AnyRef // or scala.Immutable / whatever...
}

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 3, 2012

@retronym said:
@paul: I'll continue with this one tomorrow. If you've got any tips for chasing cycles, I'm all ears.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 4, 2012

@paulp said:
They're all prayer related, but it'd be interesting to look at cycles simultaneously.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 4, 2012

@retronym said:
scala/scala#1700

I believe the cycle was valid: although adding scala.AnyRef as a parent is a no-op, determining that "AnyRef" binds to said class requires interrogation of enclosing imports, which is enough to expose the cycle.

I added a neg test case to show the situation, and also rearranged the imports to avoid flying so close to the sun.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 4, 2012

@paulp said:
Entering code block in anticipation of my underscores becoming italics...

Would you agree that it's a bug that it fails even if I fully qualify everything with _root_,
e.g. _root_.scala.AnyRef? We have already established in other tickets that _root_ is barely
acknowledged internally; you can define a package named _root_, etc. But if _root_ were
handled like the reserved word that it should be, then a parent _root_.scala.AnyRef should
not require any enhanced interrogation.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 4, 2012

@retronym said:
I tried that with the same glimmer of hope. But indeed _ root _ is just a convention right now.

While we should tighten that up, I don't think we should bend over backwards for this cycle -- it's easy enough to move the imports.

Loading

@scabug
Copy link
Author

@scabug scabug commented Dec 4, 2012

@paulp said:
I don't either, I just wanted to establish it as a bug.

Loading

@scabug scabug closed this Dec 10, 2012
@scabug
Copy link
Author

@scabug scabug commented Dec 10, 2012

Loading

@scabug scabug added the has PR label Apr 7, 2017
@scabug scabug added this to the 2.10.1 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants