Skip to content

Change/treecopying #164

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

Merged
merged 44 commits into from
Oct 11, 2014
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Aug 13, 2014

Tree copiers changed so that they now accept default arguments (at least
for the ones with more than 2 arguments). Review by @DarkDimius.

This PR has been rebased over #162, should logically reviewed after than one.

odersky added 30 commits August 3, 2014 20:43
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).
Scopes are also used in overriding pairs, and there multiple types with
the same name can be entered in a scope. So the assert to the contrary
should be limited to typechecking only.
Eta-lifting picked some arbitrary base type. It turned out that i94-nada failed once we add
a product trait to case classes (in the next commit) because Eta-Kifting picked Product
as the base type, even though the target type was bounded by Monad. We now change the scheme
so that the target type is included in the lifting, in order to avoid that we lift to
useless types.
Case classes with arity <= 1 now also get a ProductN parent trait.
This is necessary because we inherit productArity and Element methods
in case classes from the ProdictN trait.

Needed to add Product0 for this, which is not defined in Scala2.x.
After erasure, former Any members become Object members.

Also, fixed some typos and added some TODOs on addBridges.
asInstance/isInstance/ensureConforms/and/or.

They replace some former "mk..." methods.
Also renamed Boolean_and/or to _&&/||, to make it conform
to naming convention for other Definition operators.
RefChecks needs both methods.
New phase for Synthetic Method generation. Scala 2.x did it in Typer, but
it's cleaner to do it in a separate phase.
If a TypeRef with an expanded name is an alias type, print the alias instead.
Now that caes classes always inherit from ProductX, we can avoid
the special case. (We need to define _1 anyway to implement Product1).
(1) set position of companion object def
(2) companions of case classes taking multiple parameter lists do not inherit from
    a function type (reason: we can't straightforwardly converyt a curried method
    with multiple parameter lists to a function value).
For overriding checks we need a concept where a val can match a def.
Normal matches does not provide this.
Previously it did this only sometimes. Now it always prefers
concrete over deferred unless the deferred info is a proper subtype
of the concrete info.
Packages should always have a single denotation, which is invariant for all transformations.
Package members should always be entered in the first phase, and should never be entered
after a given phase.

This reflects the fact that package members correspond to classfiles. Once you create a classfile, it
stays around and is available from the start of the next run. Also, we need to prevent multiple
denotation versions of packages from hanging on to stale symbols. It would not be enough to replace
a package member by a newly compiled one; if packages had multiple denotations we'd have to do this
for all of them.
Should be `base`, was the class enclosing the definition.
RefChecks is now enabled. Some of the tests had to be fixed to be refchecks-correct.
1) Add copiers with default arguments, to avoid boilerplate
2) All copiers are now curried wrt first argument (which is
   the original tree).

We already make use of the new features in cpy.DefDef, but not yet elsewhere.
Avoid mentioning arguments that are unchanged.
… 2 elements.

It's not really an abbreviation to do it for trees with fewer elements and it leads
to unncessessary syntactic variation.
@DarkDimius
Copy link
Contributor

I like this change a lot!
Don't you mind If I'll rebase it over master to get if merged faster?

@odersky
Copy link
Contributor Author

odersky commented Aug 14, 2014

Re-rebasing: The commit conflicts in a lot of ways with the previous PRs. I don't mind rebasing, but then one has to make sure that all 4 PRs are properly rebased on each other. This will be some amount of work to achieve. I think the shorter pass is to get the other PRs in speedily. As usual all of the later steps will depend on them anyway.

Those nodes that had so far a propagateType method defined
on them are automatically retyped on copying. No more
manual interventions are needed.
Their functionality is now rolled into TypedTreeCopier.
Achieved by overridanle method treeTransformPhase in TreeTransform.
This is currently by default the current phase. We should migrate it
to the one after current phase.
We now retype basically everything except leave nodes and definitions.
This provides for more robust tree copying and transformation. It also
flushed out errors in SuperAccessors (fixed by a hack, awaiting systematic
phase change there) and UnCurryTreeTransform. Uncurry is disabled for now,
will be fixed shortly.
Making cpy recompute types uncovered errors in uncurry. In a nutshell,
the intermediate Apply nodes of a curried function were ill-typed, which
caused errors produced by TypeAssigner. These nodes were eliminated down the
road, but the errors are already issued.

I did not find a good way to treat uncurry as a treetransform. Since it is rather
trivial, it did not seem warranted to make it a full transformer either. So in
the end the uncurry functionality became part of erasure.
@DarkDimius
Copy link
Contributor

Otherwise LGTM

@DarkDimius DarkDimius merged commit 7d414eb into scala:master Oct 11, 2014
@allanrenucci allanrenucci deleted the change/treecopying branch December 14, 2017 16:58
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request Mar 19, 2025
Backport "Tweak ExtensionNullifiedByMember" to 3.3 LTS
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.

2 participants