Skip to content

Simplify trees #1538

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 5 commits into from
Sep 26, 2016
Merged

Simplify trees #1538

merged 5 commits into from
Sep 26, 2016

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Sep 25, 2016

Get rid of Pair and SelectorFromTypeTree nodes. Neither had an important role to play, so the result is shorter and simpler code than what we had before.

Drop tree node class 'Pair'. It was used only in imports, where
it can easily be replaced by Thicket.

The envisaged use for generic pairs is almost sure better modelled
by a "Pair" class in Dotty's standard library.
Roll its functionality into Select. Since we can always
tell whether a tree is a type or term there is no expressiveness
gained by having a separate tree node.
Thicket has a vararg constructor, so this syntax is redundant.
@DarkDimius
Copy link
Contributor

DarkDimius commented Sep 25, 2016

👍 It would be nice to document the change that now selectors of Imports are Thickets, not Pairs.

@odersky
Copy link
Contributor Author

odersky commented Sep 25, 2016

@DarkDimius It's in the doc comment of the Import tree.

@odersky
Copy link
Contributor Author

odersky commented Sep 25, 2016

@DarkDimius Do you want to do the review?

@smarter
Copy link
Member

smarter commented Sep 25, 2016

LGTM, though it may be nicer to have an extractor for an import rename:

  object ImportRename {
    def unapply[T >: Untyped](thicket: Thicket[T]): Option[(Ident[T], Ident[T])] = thicket match {
      case Thicket((name @ Ident(_)) :: (rename @ Ident(_)) :: Nil) => Some(name, rename)
      case _ => None
    }
  }

@odersky
Copy link
Contributor Author

odersky commented Sep 26, 2016

@smarter I am a bit concerned Import selector analysis might be time-critical, that's why I preferred the non-extractor solution, because it can be optimized better.

@odersky odersky merged commit ec28ea1 into scala:master Sep 26, 2016
@allanrenucci allanrenucci deleted the simplify-trees branch December 14, 2017 19:24
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