Skip to content

Fix/#536 nested classes #544

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

Closed
wants to merge 3 commits into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 7, 2015

This fixes #536. It makes the bet that member classes are only needed for classes that are currently compiled. It also fixes the implementation of memberClasses and localClasses, which was not according to spec before.

Review by @smarter, @DarkDimius

odersky added 3 commits May 7, 2015 11:33
…y compiled.

It seems wasteful to load the member classes even of classes that are not currently compiled.
It also makes us vulnerable to any misinterpretation of Java file formats. In th particular
case of scala#536, we parsed a class an anonymous Collection$1 which was referring to the type
parameter of its enclosing class, but was not diagnosed as an inner class of the enclosing class.
…ate.

Symbols never change betwene terms and types. So we do not need to the current
denotation to decide what they are. Less forcing -> less potential for cyclic
references.
@DarkDimius
Copy link
Contributor

Otherwise LGTM, after tests pass.

def nestedClasses: List[Symbol] = memberClasses //exitingPhase(currentRun.lambdaliftPhase)(sym.memberClasses)
def memberClasses: List[Symbol] = toDenot(sym).info.memberClasses.map(_.symbol).toList

/** For currently compiled classes: All locally defined classes including local classes.
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between a "locally defined class" and a "local class"? Also what about using the description "classes defined in the current symbol scope" or something like that?

@DarkDimius
Copy link
Contributor

Merging this PR with #543

DarkDimius added a commit to dotty-staging/dotty that referenced this pull request May 8, 2015
@DarkDimius DarkDimius closed this May 8, 2015
@allanrenucci allanrenucci deleted the fix/#536-nested-classes branch December 14, 2017 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using java.util.Collections kills the compiler ("class file ... is broken, ...")
3 participants