Fix 2403: Tweak places where we look for new definition #2414
+27
−10
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
Given a sequence of nested scope (inner to outer), typedIdent has to decode when to look for definitions. Originally his was the lawas packagst scope before the owner changes. A recent fix changed it to the first scope after the owner changed. But that is wrong for package scopes because for them we first have to analyze any neted imports before considering the package scope. This commit fixes the problem by treating package scopes specially. Also, the ambiguous import message was improved.
Looks like the commit message got a bit mangled here. Also, wow, the scoping rules are more complicated than I thought! |
smarter
reviewed
May 11, 2017
| @@ -2,7 +2,7 @@ package bar { object bippy extends (Double => String) { def apply(x: Double): St | |||
| package object println { def bippy(x: Int, y: Int, z: Int) = "(Int, Int, Int)" } | |||
| object Test { | |||
| def main(args: Array[String]): Unit = { | |||
| println(bar.bippy(5.5)) // error | |||
| println(bar.bippy(5.5)) | |||
There was a problem hiding this comment.
Isn't it concerning that this is now different from what scalac does? Is there some more subtlety in the way definitions are looked up that we're missing?
|
I think this PR could benefit from some test cases to have some concrete examples of the rules, and also to compare with what scalac does. |
|
Any update on this PR? I'd like to revert the changes in squants and scalatest to work around this change. |
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.
Given a sequence of nested scope (inner to outer), typedIdent has to
decide when to look for definitions. Originally his was the last
scope before the owner changes. A recent fix changed it to the first
scope after the owner changed. But that is wrong for package scopes
because for them we first have to analyze any nested imports before
considering the package scope.
This commit fixes the problem by treating package scopes specially.
Also, the ambiguous import message was improved.
I verified that squants compiles with this PR.