Skip to content

Fix Scope.exists and some missing Inlined trees #6233

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 4 commits into from
Apr 8, 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/core/Scopes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ object Scopes {
}

/** Tests whether a predicate holds for at least one Symbol of this Scope. */
def exists(p: Symbol => Boolean)(implicit ctx: Context): Boolean = filter(p).isEmpty
def exists(p: Symbol => Boolean)(implicit ctx: Context): Boolean = filter(p).nonEmpty

/** Finds the first Symbol of this Scope satisfying a predicate, if any. */
def find(p: Symbol => Boolean)(implicit ctx: Context): Symbol = filter(p) match {
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
super.transform(tree1)
}
case Inlined(call, bindings, expansion) if !call.isEmpty =>
val callTrace = Inliner.inlineCallTrace(call.symbol, call.sourcePos)
val pos = call.sourcePos
val callTrace = Inliner.inlineCallTrace(call.symbol, pos)(ctx.withSource(pos.source))
cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(inlineContext(call)))
case tree: Template =>
withNoCheckNews(tree.parents.flatMap(newPart)) {
Expand Down
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/YCheckPositions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ class YCheckPositions extends Phases.Phase {
assert(bindings.isEmpty)
val old = sources
sources = old.tail
traverse(expansion)(inlineContext(EmptyTree))
traverse(expansion)(inlineContext(EmptyTree).withSource(sources.head))
sources = old
case Inlined(call, bindings, expansion) =>
bindings.foreach(traverse(_))
sources = call.symbol.topLevelClass.source :: sources
if (!isMacro(call)) // FIXME macro implementations can drop Inlined nodes. We should reinsert them after macro expansion based on the positions of the trees
traverse(expansion)(inlineContext(call))
traverse(expansion)(inlineContext(call).withSource(sources.head))
sources = sources.tail
case _ => traverseChildren(tree)
}
Expand All @@ -57,8 +57,9 @@ class YCheckPositions extends Phases.Phase {
}

private def isMacro(call: Tree)(implicit ctx: Context) = {
if (ctx.phase <= ctx.typerPhase.next) call.symbol.is(Macro)
else (call.symbol.unforcedDecls.exists(_.is(Macro)) || call.symbol.unforcedDecls.toList.exists(_.is(Macro)))
if (ctx.phase <= ctx.postTyperPhase) call.symbol.is(Macro)
else call.isInstanceOf[Select] // The call of a macro after typer is encoded as a Select while other inlines are Ident
// TODO remove this distinction once Inline nodes of expanded macros can be trusted (also in Inliner.inlineCallTrace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the method alone, call.isInstanceOf[Select] seems to be a too weak test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a heuristic to know if we should YCheck the positions inside this Inlined tree or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At some point we will test the contents of these Inlined trees. Then this test would be removed.

}

}
12 changes: 7 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,12 @@ object Inliner {
* - the call's position
* in the call field of an Inlined node.
* The trace has enough info to completely reconstruct positions.
* Note: For macros it returns a Select and for other inline methods it returns an Ident (this distinction is only temporary to be able to run YCheckPositions)
*/
def inlineCallTrace(callSym: Symbol, pos: SourcePosition)(implicit ctx: Context): Tree = {
implicit val src = pos.source
Ident(callSym.topLevelClass.typeRef).withSpan(pos.span)
assert(ctx.source == pos.source)
if (callSym.is(Macro)) ref(callSym.topLevelClass.owner).select(callSym.topLevelClass.name).withSpan(pos.span)
else Ident(callSym.topLevelClass.typeRef).withSpan(pos.span)
}
}

Expand Down Expand Up @@ -250,7 +252,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
* @param bindingsBuf the buffer to which the definition should be appended
*/
private def paramBindingDef(name: Name, paramtp: Type, arg: Tree,
bindingsBuf: mutable.ListBuffer[ValOrDefDef]): ValOrDefDef = {
bindingsBuf: mutable.ListBuffer[ValOrDefDef])(implicit ctx: Context): ValOrDefDef = {
val argtpe = arg.tpe.dealiasKeepAnnots
val isByName = paramtp.dealias.isInstanceOf[ExprType]
var inlineFlag = InlineProxy
Expand Down Expand Up @@ -694,15 +696,15 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
val argSyms = (mt.paramNames, mt.paramInfos, args).zipped.map { (name, paramtp, arg) =>
arg.tpe.dealias match {
case ref @ TermRef(NoPrefix, _) => ref.symbol
case _ => paramBindingDef(name, paramtp, arg, bindingsBuf).symbol
case _ => paramBindingDef(name, paramtp, arg, bindingsBuf)(ctx.withSource(cl.source)).symbol
}
}
val expander = new TreeTypeMap(
oldOwners = ddef.symbol :: Nil,
newOwners = ctx.owner :: Nil,
substFrom = ddef.vparamss.head.map(_.symbol),
substTo = argSyms)
seq(bindingsBuf.toList, expander.transform(ddef.rhs))
Inlined(ddef, bindingsBuf.toList, expander.transform(ddef.rhs))
case _ => tree
}
case _ => tree
Expand Down
5 changes: 5 additions & 0 deletions tests/run/i4431-b/quoted_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import scala.quoted._

object Macros {
inline def h(f: => Int => String): String = f(42)
}
8 changes: 8 additions & 0 deletions tests/run/i4431-b/quoted_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import scala.quoted._
import Macros._

object Test {
def main(args: Array[String]): Unit = {
println(h(x => "abc" + x))
}
}