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

SI-6745 Skip refinements in the search for <init> #1689

Closed
wants to merge 1 commit into from

Conversation

retronym
Copy link
Member

@retronym retronym commented Dec 2, 2012

These arise in the base class sequence when a self type
is annotated. This prevented an auxillary constructor from
calling the primary constructor.

@retronym
Copy link
Member Author

retronym commented Dec 2, 2012

Review by @adriaanm

@xeno-by
Copy link
Member

xeno-by commented Dec 2, 2012

Why did the error message mention Predef?

@retronym
Copy link
Member Author

retronym commented Dec 2, 2012

Unable to find the primary constructor, it must have continued on to look for <init> in enclosing imports. I suppose we could harden against this in typedIdent and/or Context#importedSymbol.

@paulp
Copy link
Contributor

paulp commented Dec 3, 2012

Even if it doesn't materially alter the outcome, one's confidence in the compiler having some idea what it is doing cannot help but be weakened by messages like "too many arguments for constructor Predef: ()type".

@paulp
Copy link
Contributor

paulp commented Dec 3, 2012

In some apparently uncommitted branch, I have an "isImportableSymbol" which is used in Context and which excludes constructors among other things.

@paulp
Copy link
Contributor

paulp commented Dec 3, 2012

Nope, not unpublished.

    /** Is this symbol unimportable? Unimportable symbols include:
     *  - constructors, because <init> is not a real name
     *  - private[this] members, which cannot be referenced from anywhere else
     *  - members of Any or Object, because every instance will inherit a
     *    definition which supersedes the imported one
     */
    def isUnimportable(sym: Symbol) = (
         (sym eq NoSymbol)
      || sym.isConstructor
      || sym.isPrivateLocal
      || isUniversalMember(sym)
    )
    def isImportable(sym: Symbol) = !isUnimportable(sym)

@paulp
Copy link
Contributor

paulp commented Dec 3, 2012

It seems allImportedSymbols is filtered by the above (because it calls importableMembers) but importedSymbol is not.

@retronym
Copy link
Member Author

retronym commented Dec 5, 2012

This patch is still wrong with a self type other than Any. I'll fix this post haste, and then we should knock off https://issues.scala-lang.org/browse/SI-4460 too.

These arise in the base class sequence when a self type
is annotated. This prevented an auxillary constructor from
calling the primary constructor.

Unlike the first version of this change, here we do this
`typedIdent`, where we can easily determine the enclosing class.

Also closes SI-4460, which showed a more insidious version of
this bug that compiled with invalid bytecode.
@ghost ghost assigned adriaanm Dec 5, 2012
@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1069/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/1069/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1779/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1779/

@retronym
Copy link
Member Author

PLS REBUILD pr-scala-testsuite-linux-opt

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1838/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1838/

@@ -4883,7 +4883,8 @@ trait Typers extends Modes with Adaptations with Tags {
}
else {
cx = cx.enclClass
val foundSym = pre.member(name) filter qualifies
val pre1 = if (name == nme.CONSTRUCTOR) pre.baseType(context.enclClass.owner) else pre // SI-6745, SI-4460
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i worry the shape of pre and the owner chain of the current enclosing class need not always coincide but can't come up with an example

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more conceptually, I feel the prefix should've come in typed correctly in the first place
I'll try to come up with a more concrete suggestion after i've made a breadth-first sweep through my other PRs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really should have restructured things to detect name == nme.CONSTRUCTOR way earlier (https://github.com/retronym/scala/blob/df570f08/src/compiler/scala/tools/nsc/typechecker/Typers.scala#L4820) and then perform this and only this lookup.

@retronym
Copy link
Member Author

Retargetted to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants