Streamline logic related to accessor derivation in MethodSynthesis & Namers #4709
Conversation
|
This is in preparation of https://github.com/adriaanm/scala/tree/traits-late-fields, which will ultimately enable the interfaces-with-default-methods encoding of traits. |
| tree.symbol = ( | ||
| if (mods.isLazy) { | ||
| // TODO: why is all this symbol creation disconnected from addDerivedTrees, | ||
| // which creates another list of Field/Getter/Setter factories??? |
There was a problem hiding this comment.
Looks like this part enters the symbols (which needs to be done before typechecking), and addDerivedTrees is called to create the corresponding trees during typechecking.
I agree it would be cleaner to keep track of the factories we create here (maybe as a attachment on the ValDef's symbol?) and avoid the duplicated logic in allValDefDerived.
There was a problem hiding this comment.
- I'll include your comment to improve on my snarky rhetoric.
|
Progress? EDIT: nope, bug in my refactoring (in |
adriaanm
force-pushed
the
namers-accessors
branch
from
1db8a53
to
4139bd0
Compare
|
I rebased, addressing some feedback in earlier commits, adding the assert last for easy reversal. |
Give Getter control over whether a setter is needed. For now, only mutable ValDefs entail setters. In the new trait encoding, a trait val will also receive a setter from the start. Similarly, distinguish whether to derive a field from deferredness of the val. (Later, fields will not be emitted for traits, deferred or not.)
Originally (modulo renaming & reduction of double negation in previous commit):
```
def deriveAccessors(vd: ValDef) = vd.mods.isLazy || !(
!owner.isClass
|| (vd.mods.isPrivateLocal && !vd.mods.isCaseAccessor) // this is an error -- now checking first
|| (vd.name startsWith nme.OUTER)
|| (context.unit.isJava) // pulled out to caller
|| isEnumConstant(vd)
)
def deriveAccessorTrees(vd: ValDef) = !(
(vd.mods.isPrivateLocal && !vd.mods.isLazy) // lazy was pulled out to outer disjunction
|| vd.symbol.isModuleVar // pulled out to caller
|| isEnumConstant(vd))
```
With changes in comments above, these conditions are now captured by one method.
adriaanm
force-pushed
the
namers-accessors
branch
from
4139bd0
to
df61ab6
Compare
|
Running community build on merge of #4723 onto this one: https://scala-ci.typesafe.com/view/scala-2.12.x/job/scala-2.12.x-integrate-community-build/153/console |
|
LGTM |
Streamline logic related to accessor derivation in MethodSynthesis & Namers
Distinguish abstract ValDef from ValDef that needs a setter.
(Later, all trait vals will also need a setter -- right now, it's only mutable ones.)
Similarly, make explicit whether a field is needed. (Later, fields will not be emitted for traits.)
Simplify decision whether to derive accessors. Comments in the original code below (modulo renaming & reduction of double negation in previous commit) explain why I believe my changes make sense:
These conditions are now captured by one method.
Review by @lrytz, @retronym & @SethTisue. I'll check bytecode diffs (should be empty) later tonight.