Fix/refs to inner objects #633
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A reference to an object from anywhere in its module class can be established by the This of the module class. The previous behavior always referenced the object as a term ref which might cause a reference to the outer This which might not be available (since this is not tracked by ExplicitOuter). Brings t3174.scala back from disabled.
The tests in this commit pos were verified to work again.
| case pre => SelectFromTypeTree(TypeTree(pre), tp) | ||
| } // no checks necessary | ||
|
|
||
| def ref(sym: Symbol)(implicit ctx: Context): Tree = | ||
| ref(NamedType(sym.owner.thisType, sym.name, sym.denot)) | ||
|
|
||
| private def correctForPath(t: Tree)(implicit ctx: Context) = t match { |
There was a problem hiding this comment.
What does For in the name stand for?
As this method is used only in ref and is private anyway, maybe it's better to move its definition to ref and give it a proper name, eg followOuterLinks?
|
Otherwise LGTM |
Trying to diagnose failure of t2503 and t5577 which were moved to disabled in the time the tests did not work. These failed locally for me, but the checkin tests succeeded (since the checkin tests tested something else). Added better diagnostics to RefChecks which now mention candidate implementations of missing abstract members that have the right name but not the right type.
It's easy to get this wrong. What happened was that when computing deferred members of a class a deferred member was preferred over a concrete one because the types did not match. Thsi should not happen. We now change the scheme to always prefer concrete over abstract, and subclass-owned over superclass-owned. But we pick a denotation only if the overrides relationship on types coincides with the preference on symbols.
|
Since I am in train bringing back disabled tests, here's one more. AFAIK that's all the tests we had to disable in the past weeks, so we should be good on that side for now. |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several formerly disabled tests are brought back. One required fixes of references to inner objects. Review by @DarkDimius