Constructors: fixes to maintain needed private fields. #774
Conversation
Allows not to go deep into tree, additionally fixes bugs with fields that are only used in inner classes being removed.
Curiously enough, it only found bugs in old scheme It was deleting accessors, that used to be vals but became defs without anybody raising the `Method` flag.
DarkDimius
changed the title
/rebuild |
|
@odersky please review |
| if (inConstructor && (sym.is(ParamAccessor) || seenPrivateVals.contains(sym))) { | ||
| // used inside constructor, accessed on this, | ||
| // could use constructor argument instead, no need to retain field | ||
| println("hoha") |
There was a problem hiding this comment.
hoha! You probably don't want to keep this :)
| if (mightBeDropped(sym) && sym.owner.isClass) { | ||
| val owner = sym.owner.asClass | ||
|
|
||
| tree match { |
There was a problem hiding this comment.
The return value of this match is not used anywhere.
There was a problem hiding this comment.
this match returns Unit. How could it be used?
There was a problem hiding this comment.
Ah ok, I missed that retain had side-effects, I suggest replacing def retain by def retain() and using retain() to follow the scala style.
DarkDimius
force-pushed
the
fix-constructors
branch
from
5852560 to
d1ecc22
Compare
/rebuild |
|
/rebuild |
| def retain = | ||
| retainedPrivateVals.add(sym) | ||
|
|
||
| if (mightBeDropped(sym) && sym.owner.isClass) { |
There was a problem hiding this comment.
I'd reverse the order of the two conditions in the &&. mightBeDropped makes sense only for class members.
|
Otherwise LGTM |
DarkDimius
force-pushed
the
fix-constructors
branch
from
2ee4caa to
863d72d
Compare
Constructors: fixes to maintain needed private fields.
fixes #772