Skip to content

Move Staging to Typer #5846

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 17 commits into from
Mar 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Compiler {
/** Phases dealing with the frontend up to trees ready for TASTY pickling */
protected def frontendPhases: List[List[Phase]] =
List(new FrontEnd) :: // Compiler frontend: scanner, parser, namer, typer
List(new Staging) :: // Check PCP, heal quoted types and expand macros
List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
List(new PostTyper) :: // Additional checks and cleanups after type checking
List(new sbt.ExtractAPI) :: // Sends a representation of the API of classes to sbt via callbacks
Expand All @@ -45,7 +46,6 @@ class Compiler {

/** Phases dealing with TASTY tree pickling and unpickling */
protected def picklerPhases: List[List[Phase]] =
List(new Staging) :: // Check PCP, heal quoted types and expand macros
List(new Pickler) :: // Generate TASTY info
List(new ReifyQuotes) :: // Turn quoted trees into explicit run-time data structures
Nil
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/Driver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import dotty.tools.FatalError
import config.CompilerCommand
import core.Comments.{ContextDoc, ContextDocstrings}
import core.Contexts.{Context, ContextBase}
import core.{Mode, TypeError}
import core.{MacroClassLoader, Mode, TypeError}
import reporting._

import scala.util.control.NonFatal
Expand Down Expand Up @@ -54,6 +54,7 @@ class Driver {
val ctx = rootCtx.fresh
val summary = CompilerCommand.distill(args)(ctx)
ctx.setSettings(summary.sstate)
MacroClassLoader.init(ctx)

if (!ctx.settings.YdropComments.value(ctx) || ctx.mode.is(Mode.ReadComments)) {
ctx.setProperty(ContextDoc, new ContextDocstrings)
Expand Down
9 changes: 0 additions & 9 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1325,15 +1325,6 @@ object desugar {
val desugared = tree match {
case SymbolLit(str) =>
Literal(Constant(scala.Symbol(str)))
case Quote(t) =>
if (t.isType)
TypeApply(ref(defn.QuotedType_applyR), List(t))
else
Apply(ref(defn.QuotedExpr_applyR), t)
case Splice(expr) =>
Select(expr, nme.splice)
case TypSplice(expr) =>
Select(expr, tpnme.splice)
case InterpolatedString(id, segments) =>
val strs = segments map {
case ts: Thicket => ts.trees.head
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,13 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
foreachSubTree { tree => if (f(tree)) buf += tree }
buf.toList
}

/** Set this tree as the `defTree` of its symbol and return this tree */
def setDefTree(implicit ctx: Context): ThisTree = {
val sym = tree.symbol
if (sym.exists) sym.defTree = tree
tree
}
}

/** Map Inlined nodes, NamedArgs, Blocks with no statements and local references to underlying arguments.
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,9 @@ object Flags {
/** An inline method */
final val InlineMethod: FlagConjunction = allOf(Inline, Method)

/** An inline by-name parameter proxy */
final val InlineByNameProxy: FlagConjunction = allOf(InlineProxy, Method)

/** An inline parameter */
final val InlineParam: FlagConjunction = allOf(Inline, Param)

Expand Down
26 changes: 26 additions & 0 deletions compiler/src/dotty/tools/dotc/core/MacroClassLoader.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package dotty.tools.dotc.core

import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.util.Property
import dotty.tools.dotc.reporting.trace

import scala.collection.mutable

object MacroClassLoader {

/** A key to be used in a context property that caches the class loader used for macro expansion */
private val MacroClassLoaderKey = new Property.Key[ClassLoader]

/** Get the macro class loader */
def fromContext(implicit ctx: Context): ClassLoader =
ctx.property(MacroClassLoaderKey).getOrElse(makeMacroClassLoader)

/** Context with a cached macro class loader that can be accessed with `macroClassLoader` */
def init(ctx: FreshContext): ctx.type =
ctx.setProperty(MacroClassLoaderKey, makeMacroClassLoader(ctx))

private def makeMacroClassLoader(implicit ctx: Context): ClassLoader = trace("new macro class loader") {
val urls = ctx.settings.classpath.value.split(java.io.File.pathSeparatorChar).map(cp => java.nio.file.Paths.get(cp).toUri.toURL)
new java.net.URLClassLoader(urls, getClass.getClassLoader)
}
}
25 changes: 25 additions & 0 deletions compiler/src/dotty/tools/dotc/core/StagingContext.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package dotty.tools.dotc.core

import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.util.Property

import scala.collection.mutable

object StagingContext {

/** A key to be used in a context property that tracks the quoteation level */
private val QuotationLevel = new Property.Key[Int]

/** All enclosing calls that are currently inlined, from innermost to outermost. */
def level(implicit ctx: Context): Int =
ctx.property(QuotationLevel).getOrElse(0)

/** Context with an incremented quotation level. */
def quoteContext(implicit ctx: Context): Context =
ctx.fresh.setProperty(QuotationLevel, level + 1)

/** Context with a decremented quotation level. */
def spliceContext(implicit ctx: Context): Context =
ctx.fresh.setProperty(QuotationLevel, level - 1)

}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ object PickledQuotes {
case Block(stats, expr) =>
seq(stats, rec(expr)).withSpan(fn.span)
case _ =>
fn.select(nme.apply).appliedToArgs(argRefs())
fn.select(nme.apply).appliedToArgs(argRefs()).withSpan(fn.span)
}
Block(argVals, rec(fn))
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -851,8 +851,6 @@ class TreeUnpickler(reader: TastyReader,
sym.info = ta.avoidPrivateLeaks(sym, tree.sourcePos)
}

sym.defTree = tree

if (ctx.mode.is(Mode.ReadComments)) {
assert(ctx.docCtx.isDefined, "Mode is `ReadComments`, but no `docCtx` is set.")
commentUnpicklerOpt.foreach { commentUnpickler =>
Expand All @@ -862,7 +860,7 @@ class TreeUnpickler(reader: TastyReader,
}
}

tree
tree.setDefTree
}

private def readTemplate(implicit ctx: Context): Template = {
Expand Down
219 changes: 219 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
package dotty.tools.dotc
package transform

import dotty.tools.dotc.ast.Trees._
import dotty.tools.dotc.ast.{TreeTypeMap, tpd, untpd}
import dotty.tools.dotc.core.Constants._
import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.core.Decorators._
import dotty.tools.dotc.core.Flags._
import dotty.tools.dotc.core.quoted._
import dotty.tools.dotc.core.NameKinds._
import dotty.tools.dotc.core.StagingContext._
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.Symbols._
import dotty.tools.dotc.core.tasty.TreePickler.Hole
import dotty.tools.dotc.core.Types._
import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.util.Spans._
import dotty.tools.dotc.transform.SymUtils._
import dotty.tools.dotc.transform.TreeMapWithStages._
import dotty.tools.dotc.typer.Implicits.SearchFailureType
import dotty.tools.dotc.typer.Inliner

import scala.collection.mutable
import dotty.tools.dotc.util.SourcePosition

import scala.annotation.constructorOnly

/** Checks that the Phase Consistency Principle (PCP) holds and heals types.
*
* Type healing consists in transforming a phase inconsistent type `T` into a splice of `implicitly[Type[T]]`.
*/
class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(ictx) {
import tpd._

override def transform(tree: Tree)(implicit ctx: Context): Tree = tree match {
case tree: DefDef if tree.symbol.is(Inline) && level > 0 => EmptyTree
case _ => checkLevel(super.transform(tree))
}

/** Transform quoted trees while maintaining phase correctness */
override protected def transformQuotation(body: Tree, quote: Tree)(implicit ctx: Context): Tree = {
val body1 = transform(body)(quoteContext)
super.transformQuotation(body1, quote)
}

/** Transform splice
* - If inside a quote, transform the contents of the splice.
* - If inside inlined code, expand the macro code.
* - If inside of a macro definition, check the validity of the macro.
*/
protected def transformSplice(splice: Select)(implicit ctx: Context): Tree = {
if (level >= 1) {
val body1 = transform(splice.qualifier)(spliceContext)
val splice1 = cpy.Select(splice)(body1, splice.name)
if (splice1.isType) splice1
else addSpliceCast(splice1)
}
else {
assert(!enclosingInlineds.nonEmpty, "unexpanded macro")
assert(ctx.owner.isInlineMethod)
if (Splicer.canBeSpliced(splice.qualifier)) { // level 0 inside an inline definition
transform(splice.qualifier)(spliceContext) // Just check PCP
splice
}
else { // level 0 inside an inline definition
ctx.error(
"Malformed macro call. The contents of the splice ${...} must call a static method and arguments must be quoted or inline.",
splice.sourcePos)
splice
}
}
}


/** Add cast to force boundaries where T and $t (an alias of T) are used to ensure PCP.
* '{ ${...: T} } --> '{ ${...: T}.asInstanceOf[T] } --> '{ ${...: T}.asInstanceOf[$t] }
*/
protected def addSpliceCast(tree: Tree)(implicit ctx: Context): Tree = {
val tp = checkType(tree.sourcePos).apply(tree.tpe.widenTermRefExpr)
tree.cast(tp).withSpan(tree.span)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it always have to be a cast? Or can one use ensureConforms instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not always need to cast, but ensureConforms would not work here because we always cast to a type that is equal modulo spliced type aliases.

A recording of the splice into something that is not the type alias $splice in quoted.Type could avoid this cast altogether. I intend to do this in a future PR, I will need it to hide internal encoding from the user and in #5297.

}

/** If `tree` refers to a locally defined symbol (either directly, or in a pickled type),
* check that its staging level matches the current level. References to types
* that are phase-incorrect can still be healed as follows:
*
* If `T` is a reference to a type at the wrong level, try to heal it by replacing it with
* `${implicitly[quoted.Type[T]]}`.
*/
protected def checkLevel(tree: Tree)(implicit ctx: Context): Tree = {
def checkTp(tp: Type): Type = checkType(tree.sourcePos).apply(tp)
tree match {
case Quoted(_) | Spliced(_) =>
tree
case tree: RefTree if tree.symbol.is(InlineParam) =>
tree
case _: This =>
assert(checkSymLevel(tree.symbol, tree.tpe, tree.sourcePos).isEmpty)
tree
case _: Ident =>
checkSymLevel(tree.symbol, tree.tpe, tree.sourcePos) match {
case Some(tpRef) => tpRef
case _ => tree
}
case _: TypeTree | _: AppliedTypeTree | _: Apply | _: TypeApply | _: UnApply | Select(_, OuterSelectName(_, _)) =>
tree.withType(checkTp(tree.tpe))
case _: ValOrDefDef | _: Bind =>
tree.symbol.info = checkTp(tree.symbol.info)
tree
case _: Template =>
checkTp(tree.symbol.owner.asClass.givenSelfType)
tree
case _ =>
tree
}
}

/** Check and heal all named types and this-types in a given type for phase consistency. */
private def checkType(pos: SourcePosition)(implicit ctx: Context): TypeMap = new TypeMap {
def apply(tp: Type): Type = reporting.trace(i"check type level $tp at $level") {
tp match {
case tp: TypeRef if tp.symbol.isSplice =>
if (tp.isTerm)
ctx.error(i"splice outside quotes", pos)
tp
case tp: NamedType =>
checkSymLevel(tp.symbol, tp, pos) match {
case Some(tpRef) => tpRef.tpe
case _ =>
if (tp.symbol.is(Param)) tp
else mapOver(tp)
}
case tp: ThisType =>
assert(checkSymLevel(tp.cls, tp, pos).isEmpty)
mapOver(tp)
case _ =>
mapOver(tp)
}
}
}

/** Check reference to `sym` for phase consistency, where `tp` is the underlying type
* by which we refer to `sym`. If it is an inconsistent type try construct a healed type for it.
*
* @return `None` if the phase is correct or cannot be healed
* `Some(tree)` with the `tree` of the healed type tree for `${implicitly[quoted.Type[T]]}`
*/
private def checkSymLevel(sym: Symbol, tp: Type, pos: SourcePosition)(implicit ctx: Context): Option[Tree] = {
/** Is a reference to a class but not `this.type` */
def isClassRef = sym.isClass && !tp.isInstanceOf[ThisType]

if (sym.exists && !sym.isStaticOwner && !isClassRef && !levelOK(sym))
tryHeal(sym, tp, pos)
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic here is very hard to follow, to the point where I cannot be sure it is correct.
A clearer formulation would be:

val needsCheck =
  tp.isInstanceOf[ThisType] || sym.is(Param) || sym.owner.isTerm
if (sym.exists && needsCheck && !sym.isStaticOwner && !levelOK(sym)) 
  tryHeal(sym, tp, pos)
else
  None

and it would be good to have further explanations in comments.

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 partially refactored it.

Copy link
Contributor

Choose a reason for hiding this comment

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

shouldtryHealable can be a def, since it is only called after 3 other conditions.
It would be good to add a short comment to each of the 3 lines of shouldTryHealable, indicating why we
should try to heal in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After playing around with this condition I figured out that is was checking is the symbol is a reference to a class (but not a this.type). I updated the code accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that clarifies things!

else
None
}

/** Does the level of `sym` match the current level?
* An exception is made for inline vals in macros. These are also OK if their level
* is one higher than the current level, because on execution such values
* are constant expression trees and we can pull out the constant from the tree.
*/
private def levelOK(sym: Symbol)(implicit ctx: Context): Boolean = levelOf(sym) match {
case Some(l) =>
l == level ||
level == -1 && (
sym == defn.TastyReflection_macroContext ||
// here we assume that Splicer.canBeSpliced was true before going to level -1,
// this implies that all non-inline arguments are quoted and that the following two cases are checked
// on inline parameters or type parameters.
sym.is(Param) ||
sym.isClass // reference to this in inline methods
)
case None =>
!sym.is(Param) || levelOK(sym.owner)
}

/** Try to heal phase-inconsistent reference to type `T` using a local type definition.
* @return None if successful
* @return Some(msg) if unsuccessful where `msg` is a potentially empty error message
* to be added to the "inconsistent phase" message.
*/
protected def tryHeal(sym: Symbol, tp: Type, pos: SourcePosition)(implicit ctx: Context): Option[Tree] = {
def levelError(errMsg: String) = {
def symStr =
if (!tp.isInstanceOf[ThisType]) sym.show
else if (sym.is(ModuleClass)) sym.sourceModule.show
else i"${sym.name}.this"
ctx.error(
em"""access to $symStr from wrong staging level:
| - the definition is at level ${levelOf(sym).getOrElse(0)},
| - but the access is at level $level.$errMsg""", pos)
None
}
tp match {
case tp: TypeRef =>
if (level == -1) {
assert(ctx.inInlineMethod)
None
} else {
val reqType = defn.QuotedTypeType.appliedTo(tp)
val tag = ctx.typer.inferImplicitArg(reqType, pos.span)
tag.tpe match {
case fail: SearchFailureType =>
levelError(i"""
|
| The access would be accepted with the right type tag, but
| ${ctx.typer.missingArgMsg(tag, reqType, "")}""")
case _ =>
Some(tag.select(tpnme.splice))
}
}
case _ =>
levelError("")
}
}

}
Loading