Skip to content

Fix #1568 - avoid transforming error trees #1930

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
Feb 2, 2017

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 2, 2017

If a tree has type error, subtrees may not have an assigned type.
Therefore we should avoid transforming such trees.

If a tree has type error, subtrees may not have an assigned type.
Therefore we should avoid transforming such trees.
@@ -573,6 +573,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
Try(tree: Tree)(expr, cases, finalizer)
}

override def skipTransform(tree: Tree)(implicit ctx: Context) = tree.tpe.isError
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be cleaner to just override the transform as follows:

override def transform(tree: Tree)(implicit ctx: Context): Tree = 
  if (tree.tpe.isError) tree else super.transform(tree)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was debating what was simpler, and more efficient. I also first thought of overriding transform, but: (1) this requires a new TreeMap class in tpd (2) this adds a stackframe to each node traversal, which can add up. So I think the current approach is more lightweight in the end.

@nicolasstucki
Copy link
Contributor

Otherwise it LGTM

@odersky odersky merged commit 10bf4ee into scala:master Feb 2, 2017
@allanrenucci allanrenucci deleted the fix-#1568 branch December 14, 2017 16:59
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