Transform/flatten #203
Closed
Transform/flatten #203
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's often useful to print a value as it is returned. This method
avoids the temporary variable to hold the vaue before it is printed
and then return. I.e.
printer.echo(msg, expr)
instead of
val x = expr
printer.println(msg + x)
x
The patch disables hoisting of classes local to a block into the result type of the block. Instead, we widen the result type of the block to one which reflects all refinements made to the parents type of the local class. Test cases in avoid.scala, t1569.scala. The original t1569.scala no longer works. Why is explained in neg/t1569-failedAvoid.scala
Flushed out a caching bug in Scopes.
Package object members are seen as members of the enclosing package during typer. The normalization inserts the missing .package reference to such members. It is necessary to satisfy a new postcondition of FirstTransform: In a selection `x.m`, the type of `x` must derive from the owner of `m`.
... and these mappings have to be part of the applied substitutions. Without the patch, the postCondition of FirstTransform fails for TreeInfo.scala and others, because it selects symbols which are not defined in the mapped class. Unrelated bugfix: JavaArray derives from Object.
Now handles the case where a class symbol itself is not changed by the map, but one of its declarations is. In this case we need to back out, and create new symbols for the class and all other symbols that are defined in the same scope as the class.
... by forwarding to Object. Without this, LambdaLift fails for core/Contexts.scala.
Static has two meanings: In the Java sense, and in the Scala sense, where it means a member of a package or static module. The change makes it clear that the flag means Static in the Java sense.
Statement sequences need to be flattened after processing, not before.
typeParams and outerAccessor both potentially scan all declarations of a class. The fixes make sure this is never done for packages.
The skip logic in enclosing class worked only when the original symbol was labelled inSuperCall. The patch makes it work also for symbols that are in turn owned by an inSuperCall symbol. Also it treats JavaStatic terms as also not being enclosed by the lexically enclosing class.
Several fixes to LambdaLift. The test suite now succeeds with LambdaLift enabled.
Nested methods cannot refer to labels in theior environment. Needs a fix in TailCalls. Moved failing test to pending.
- supertype components needs to be recursively erased Without this fix, some files do not pass -Ycheck:lambdaLift
All tests pass, but good to have the condition in there.
... so that they do not spill over between compilation units. It would be better to wipe the maps after processing a compilation unit, but right now we do not have a hook for that. A better solution should be possible once we replace init by "prepareUnit/transformUnit".
If a symbol becomes a class field, references to it need to be selections, or else we get a "bad type" assertion violation in TreeChecker.
Only exception: dotc/transform. This seems to be for two reasons: 1) The call-by-name functions used in Decorator#foldRightBN cannot be translated correctly at their use points. 2) An anonymous function in Constructors are not correctly lifted. 2) could be related to missing/duplicated symbols in pattern matcher. I'll follow up with a commit that points these out.
TreeChecker now tests that a symbol does not have two definitions that define it, and that every reference to a symbol owner by a term is in the scope of a definition of that symbol. Both tests fail on several files for pattern matcher.
…re known to succeed.
The original problem was that in an expression
f(x = bar(y = z))
only the outer named arg was eliminated by FirstTransform.
The first error was that the postcondition in FirstTransform did not get to the named arg, because
it got called from the overrdden typed method in TreeChecker, yet function arguments were evaluated
with typedUnadapted.
action: change Retyper and TreeChecker to override typedUndapped instead of typed.
This flushed out the second error: transformOther in FirstTransform needs to recursively transform the argument
of a NamedArg, because the framework itself does not handle NamedArg nodes.
Now, all tests pass except that TreeChecker itself fails -Ycheck:gettersSetters due to a problem
with handling by-name function types. This should be fixed in a separate PR.
By-name functions like `(=> T) => T` were not treated correctly before. Witness the disabled `-Ycheck:gettersSetters` for transform/TreeCheckers in thge test suite. This commit changes the scheme how => T types are treated and fixes the problems with by-name functions.
Some changes needed so that Flatten can run after LambdaLift
To be combinable with follow-up mini-phases the lift operation needs to handle Thickets specially. This commit factors out the behavior from LambdaLift, so that Flatten can do the same thing.
Cleans up after LambdaLift and Flatten. RestoreScopes exhibited a problem (double definition) when compiling Unpickler. The root of the problem was in Applications.scala. The effect was that arguments woulkd be lifted out, but then the argument expression would be used anyway. That caused a closure to be present twice which caused the double def error much later. -Ycheck did not catch it because the two closure expressions were in non-overlapping scopes.
|
Depends on #202. |
|
Review by @olhotak or @DarkDimius or anybody else who wants to have a look. |
This was referenced Oct 29, 2014
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two more miniphases: Flatten and RestoreScopes.