Skip to content

Tree transformer&TreeTransform #43

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 1 commit into from
Mar 6, 2014

Conversation

DarkDimius
Copy link
Contributor

To speedup discussion I propose my implementation

@xeno-by
Copy link

xeno-by commented Mar 4, 2014

What about prepareXXX methods?


def transformAnnotated(tree: Annotated, annot: Tree, arg: Tree)(implicit ctx: Context): Annotated = cpy.Annotated(tree, annot, arg)

def transformSharedTree(tree: SharedTree, shared: Tree)(implicit ctx: Context): SharedTree = cpy.SharedTree(tree, shared)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going away?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially yes. But now it's still here.

@DarkDimius
Copy link
Contributor Author

@odersky I had a close look at optimizations in your implementation and incorporated similar ideas:

  1. using fast tracks in case node type isn't altered by Transformation
  2. using pre-computed hints to quickly jump to next transformation interested in transforming particular Tree type
  3. using pre-computed hints to know which transformations are going to 'prepare' for transforming particular Tree type
  4. recomputing those hints in case some transformation changed implementation class during 'prepare'

tree = tree match {
case tree: Ident =>
var t: Tree = tree
val mutatedInfo = mutateTransformers(info, _.prepareForIdent(tree), info.nx.nxPrepIdent, cur)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idea: preemptively create mutators

@DarkDimius
Copy link
Contributor Author

Result of discussion: split big method, have transformers responsible of calling remaining transformations on created nodes.

@DarkDimius
Copy link
Contributor Author

@odersky @sjrd please review.

@sjrd
Copy link
Member

sjrd commented Mar 6, 2014

LGTM, although I suggest squashing the 4 commits because their history is erased in the last one anyway.

@odersky
Copy link
Contributor

odersky commented Mar 6, 2014

Agree on sqashing and another small thing: I would suggest to reorder prepare/transform methods so that each is a block. It's easier to see what kinds of methods are supported that way, and it's also simpler to just copy a block of transform methods in order to modify them in a subclass. Otherwise LGTM.

1) using fast tracks in case node type isn't altered by Transformation;
2) using pre-computed hints(nxTransformXXX arrays) to quickly jump to next transformation interested in transforming particular Tree type;
3) using pre-computed hints(nxPrepareXXX arrays) to know which transformations are going to 'prepare' for transforming particular Tree type;
4) recomputing those hints in case some transformation changed implementation class during 'prepare';
5) TreeTransform is now responsible of calling transformFollowing on nodes created by it.
DarkDimius added a commit that referenced this pull request Mar 6, 2014
@DarkDimius DarkDimius merged commit 172f480 into scala:master Mar 6, 2014
noti0na1 pushed a commit to noti0na1/dotty that referenced this pull request Nov 14, 2019
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request Mar 19, 2025
Backport "Apply implicit conversion from derived Conversion instance defined as implicit rather than given" 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.

4 participants