Skip to content
This repository was archived by the owner on Mar 28, 2018. It is now read-only.

Refactoring typechecker #3

Open
wants to merge 15 commits into
base: 2.11.x
Choose a base branch
from
Open

Refactoring typechecker #3

wants to merge 15 commits into from

Conversation

EECOLOR
Copy link
Owner

@EECOLOR EECOLOR commented Feb 4, 2015

Types of refactoring:

  • Optimizing dependency graph - Moves members to more appropriate places to prevent unnecessary coupling. In a lot of cases this is done to follow the Single Responsibility Principle.
  • Minimize public API - Keep the public API as small as possible. This helps with refactoring as you can more easily determine which methods you can safely change / rename. In a lot of cases this is done to follow the [Interface Segregation Principle]. As a bonus this improves incremental compilation speed.

TODO: Group commits by type (or squash completely)

import global._

private [nsc] def minimizeParents(parents: List[Type]): List[Type]
private [nsc] def javaSig(sym0: Symbol, info: Type): Option[String]
Copy link

Choose a reason for hiding this comment

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

This method is called from the IDE.

% git grep erasure.javaSig
org.scala-ide.sdt.core/src/org/scalaide/core/internal/compiler/JavaSig.scala:          for (signature <- erasure.javaSig(symbol, pc.enteringPhase(pc.currentRun.erasurePhase)(symbol.info)))

In general, be careful changing public to private[nsc] for this reason.

Once you're ready, we can your branch through our PR validation and community build that will check that the compiler clients like Scala IDE, SBT, Ensime and Macro Paradise still work.

@EECOLOR EECOLOR force-pushed the refactoring_typechecker branch from a18472f to 9b32c21 Compare February 11, 2015 21:35
`HasMember` was only used by `Typers` yet it was located in `Implicits`. The method `memberWildcardType` seemed out of place at `Implicits`. While it's used by `Implicits` it is a utility method not specialized for `Implicits`.

- Moved object `HasMember` from `Implicits` to `Typers` (the only call site) and made it private.
- Moved method `memberWildcardType` to `Types` which contains similar methods and is available to both `Typer` and `Implicits`.
`NoImplicitInfo` is only used by `Implicits`

- Made `NoImplicitInfo` in `Implicits` private.
`apply` of `HasMethodMatching` was only used by `Typers` and `unapply` of `HasMethodMatching` was only used by `Implicits`. Allthough they represent the same type of thing and can be used together they are actually not the same thing. The name `HasMethodMatching` does not match the contents of the `apply` method.

Another option would be to rename this combination to something that represent the construction. I do not think that this construction deserves it's own type as it is only used in a very limited number of places. On top of that, the unapply matches more types than are created by the apply method.

- Moved `apply` method to a new private object `MemberWildcardMethodType` in `Typers`
- Made supporting constructs `dummyMethod` and `templateArgType` private
- Made `HasMethodMatching` private
`Function1` is only used by `Implicits`

- Made `Function1` in `Implicits` private.
`check` method manually checked for presence of `msg` argument. This is already performed by the annotations compiler.

- Made `undetParams` in `Implicits` private. `undetParams` is only used by `Implicits`.
- Refactored `check` method in `ImplicitNotFoundMsg` to use `sym.ImplicitNotFoundMsg`
- Removed unused method `format` from `Message`
- Moved definition `typeArgsAtSym` into the only calling method
… typechecker parts are (where possible) on interfaces. Moved interfaces to separate files to help with incremental compilation. A change in the DefaultTyper (for example) now causes a smaller ripple in the sea of compiled files.
…led (most) implementations in typechecker from Global. Tests green
…orking because of partests dependency on Global as an implementation.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants