Fix/transforms #162
Fix/transforms #162
Conversation
These were not handled before.
Added decorators for symbols that can query specific annotations and annotation arguments (for now, -deprecated and -migration are added)
Added method to traverse all parts of a type.
Two improvements to TreeTransform: 1) Added transformOther functionality which handles trees not handled by other parts 2) Passes down Mode.Pattern in the context when in a pattern. TreeTransform no longer normalizes unknown trees but passes them to transformOther. The former Companions phase has been renamed to FirstTransform. It now performs the following optimizations: - adds companion objects (this was done before) - other normalizations that were formally done in TreeTransform, - rewrite native methods to stubs (this was formally done in RefChecks)
Cycles are now detected early, when an info is first completed. Legal, f-bounded cycles are broken by a LazyRef, which will construct its type lazily. This makes checkBounds validation of AppliedTypeTrees work (in FirstTransform). Formerly, this stackoverflowed despite the laziness precautions in findMember. Todo: Do the same for class files coming from Java and Scala 2.x.
Insert LazyRefs to break cycles for F-bounded types that are unpickled or read from Java signatures.
Now that F-bunded types are treated more robustly, we can check bounds for non-emptyness during Typer. This unvealed one wrong test (wonder how that passed scalac?), which got moved to neg.
Statements are now transformed with the transform returned by prepareStats, analogoys to the other prepare methods.
This is still disabled, because the prepare machinery in transform does not work yet. Concretely, prepare ops need to return a new TreeTransform but that tree transform has an undefined phaase id. We need some architectural changes to disentangle transforms from phases.
... so that it can be combined with TreeTransform in a trait composition in a future dientanglement of TreeTransforms and Phases.
Would be flagged as unimplemented members in refChecks otherwise
TreeTransforms are no longer phases. This allows to generate new transforms in prepare... methods without running into the problem that thee new transforms are undefined as phases. It also makes for a cleaner separation of concerns.
Should have lazy flag set, otherwise forward reference checking would fail for modules. Note: LazyVals needed to be disabled because it also should transform module vals, but didn't do this so far because it only tested the Lazy flag. It turned out the module val transformation exposed some bugs in lazy vals in that LazyVals creates symbols as a side effect and enters them into scopes. Such mutations are allowed onyl in very specific cases (essentially only for local throw-away scopes).
|
What is the rationale behind putting multiple independent transformations inside |
|
I thought about it, but think it's defensible to put these transforms together: they all canonicalize the trees. Let's keep in mind that the total number of transforms is limited. Right now it's 32 which is definitely too small. We can bump it to 64 or even 128 if we limit the number of possible runs in Period, but in any case that number is finite. I also think it would not help understandability if we split transforms into too many salami slices. |
|
Maybe at least split it into several traits? Those transformations are independent and I believe it's better to keep at least their implementations separate not to create new |
|
It's all a matter of balance. If this phase was 500+ lines, I'd agree. But the payload of this transform is 60 lines (the rest is writing it in On Wed, Aug 13, 2014 at 4:40 PM, Dmitry Petrashko [email protected]
Martin Odersky |
Backport "Documentation only: update Example code linked to obsolete content in macros-spec.md " to 3.3 LTS
More steps to make RefChecks work (an initial version is included in commit, this compiles, but does not run).
Main changes are to TreeTransforms. In particular TreeTransforms and phases are now disentangled.
The last commit makes modules lazy and disables LazyVals.
This PR depends on #161. #161 should be reviewed and committed first.
Review by @DarkDimius.