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

Wrong error: Companions Foo and Foo must be defined in the same source file #6490

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

Comments

@scabug
Copy link

@scabug scabug commented Oct 9, 2012

I spent some time debugging the spurious error we started to get recently in the presentation compiler. It is about 'Companions X and X must be defined in the same file'. You can see one user complaining here: https://groups.google.com/d/msg/scala-internals/aPGxqZSyvYg/W5yo9TKKMl4J

In my tests, I can reproduce it with any class in the Empty package (but not only there).

  • my source is just 'class Foo {}'
  • I don't define a companion myself, nor should there be one. Am I right?
  • the source files that are listed in the error message are the same

I traced it back to commit 816cecf9a95989dfc570f2acad87d4156b09e2ff. Reverting this made the problem go away (I'm not suggesting that, just a data point).
Debugging lead to the following event:

  • the immediate cause is that 'isCoDefinedWith' returns false for the two symbols
  • it returns false because the first test fails (this.rawInfo ne NoType)

NOTE: This looks asymmetrical. Is that intended? If I call 'that isCoDefinedWith this' I might get a different result..

        companionSymbolOf(sym, context) andAlso { companion =>
          val assignNoType = companion.rawInfo match {
            case _: SymLoader => true
            case tp           => tp.isComplete && (runId(sym.validTo) != currentRunId)
          }
          // pre-set linked symbol to NoType, in case it is not loaded together with this symbol.
          if (assignNoType)
            companion setInfo NoType
        }

This code makes companions that were companions (the call to companionSymbolOf just returned my companion) not be companions anymore. This is what I'm witnessing in the IDE.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Oct 9, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6490?orig=1
Reporter: @dragos
Affected Versions: 2.10.0
Other Milestones: 2.10.0

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Oct 9, 2012

@paulp said:
That logic is at least 7 years old: scala/scala@322068f

+      if (sym.owner.isPackageClass &&
+          (sym.linkedSym.rawInfo.isInstanceOf[loaders.SymbolLoader] ||
+           sym.linkedSym.rawInfo.isComplete && sym.validForRun != currentRun))
         // pre-set linked symbol to NoType, in case it is not loaded together with this symbol.
         sym.linkedSym.setInfo(NoType);
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Oct 10, 2012

@dragos said:
For future generations of compiler engineers:

  • companions for existing classes are created by the symbol loader, and they may in fact not exist. When the class is loaded, if there is no companion its companion symbol type is set to NoType (the symbol can't be wiped out anymore)

  • The logic to set it to NoType is therefore correct

  • The fix was in the validation check: before calling isCoDefinedWith, check that both symbols exist

  • isCoDefinedWith remains asymmetrical. My attempts to "fix" it hit too many errors/crashes in the compiler. It's caller's responsibility to make sure the symbols exist before calling isCoDefinedWith.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Oct 10, 2012

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Oct 10, 2012

@jsuereth said:
I believe the patch fixed the issue, so... yay.

@scabug scabug closed this Oct 10, 2012
@scabug scabug added this to the 2.10.0-M7 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.