Fix #2220: Change handling of package objects and tweak hk type inference #2205
Conversation
1. Invalidate packageObj cache when entering a package object
2. Prefer package object members over same-named package members
unless we are in the scala package
3. Exclude package objects from no-double-bindings checks, since
package objects may now be visited before indexing them.
| * | ||
| * If this is the scala package or the package object exists but is currently completing, | ||
| * look in the package first, and if nothing is found there, look in the package object second. | ||
| * Otherwise, look in the package object first, and if nothing is found there, in |
There was a problem hiding this comment.
The spec says "The package object should not define a member with the same name as one of the top-level objects or classes defined in package p. If there is a name conflict, the behavior of the program is currently undefined.", we probably want to update that at some points with the rules here.
There was a problem hiding this comment.
| else compareLower(bounds(param2), tyconIsTypeRef = false) | ||
| } | ||
| isMatchingApply(tp1) || | ||
| canConstrain(param2) && canInstantiate(param2) || |
There was a problem hiding this comment.
Just merged #2204. I don't have a separate test case for this change, but would argue that the fact that we missed the case before means we cannot exclude that we will miss it in the future. So better play it safe.
| * complete it too early, we freeze its superclass Any, so that no members can | ||
| * be entered in it. As a consequence, there should be no entry in the scala package | ||
| * object that hides a class or object in the scala package of the same name, because | ||
| * the behavior would then be unintuitive for such members. |
There was a problem hiding this comment.
Would it possible to detect when a scala package object member hides a scala class/object and return an error?
There was a problem hiding this comment.
Not sure. It would be tricky since everything we do in the scala package is fraught with causing all sorts of initialization errors. Not sure it's worth it really, given that we never had a problem so far.
There was a problem hiding this comment.
LGTM, I've tagged this PR "needs spec" so that in the future we can easily find it if we want to update http://www.scala-lang.org/files/archive/spec/2.13/09-top-level-definitions.html#package-objects (I think we should try to tag all PRs that will require spec changes, otherwise it's too easy to forget about them)
No description provided.