Skip to content

Commit

Permalink
Fix callTrace of inlined methods (#18738)
Browse files Browse the repository at this point in the history
We need to keep the reference to the called method, not only the symbol
of the to level class. This is important for the traces of the `assert`
method that is defined in a different file. This might also be useful
for macro annotations.

This is also a solution to the awkward Select vs. Ident distinction to
identify macros in `YCheckPositions`.
  • Loading branch information
odersky authored Nov 6, 2023
2 parents 319e865 + eb0c408 commit f61026d
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 23 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ jobs:
run: sbt ";dist/pack ;scala3-bootstrapped/compile ;scala3-bootstrapped/test"
shell: cmd

- name: Test with Scala 2 library TASTy
run: sbt ";set ThisBuild/Build.useScala2LibraryTasty := true ;scala3-bootstrapped/testCompilation i5" # only test a subset of test to avoid doubling the CI execution time
shell: cmd

- name: Scala.js Test
run: sbt ";sjsJUnitTests/test ;set sjsJUnitTests/scalaJSLinkerConfig ~= switchToESModules ;sjsJUnitTests/test ;sjsCompilerTests/test"
shell: cmd
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,8 @@ object Trees {
*
* @param call Info about the original call that was inlined
* Until PostTyper, this is the full call, afterwards only
* a reference to the toplevel class from which the call was inlined.
* a reference to the method or the top-level class from
* which the call was inlined.
* @param bindings Bindings for proxies to be used in the inlined code
* @param expansion The inlined tree, minus bindings.
*
Expand Down
14 changes: 0 additions & 14 deletions compiler/src/dotty/tools/dotc/inlines/Inlines.scala
Original file line number Diff line number Diff line change
Expand Up @@ -299,20 +299,6 @@ object Inlines:
(new Reposition).transform(tree)
end reposition

/** Leave only a call trace consisting of
* - a reference to the top-level class from which the call was inlined,
* - 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)(using Context): Tree = {
assert(ctx.source == pos.source)
val topLevelCls = callSym.topLevelClass
if (callSym.is(Macro)) ref(topLevelCls.owner).select(topLevelCls.name)(using ctx.withOwner(topLevelCls.owner)).withSpan(pos.span)
else Ident(topLevelCls.typeRef).withSpan(pos.span)
}

private object Intrinsics:
import dotty.tools.dotc.reporting.Diagnostic.Error
private enum ErrorKind:
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/PickleQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ object PickleQuotes {
def pickleAsTasty() = {
val body1 =
if body.isType then body
else Inlined(Inlines.inlineCallTrace(ctx.owner, quote.sourcePos), Nil, body)
else Inlined(ref(ctx.owner.topLevelClass.typeRef).withSpan(quote.span), Nil, body)
val pickleQuote = PickledQuotes.pickleQuote(body1)
val pickledQuoteStrings = pickleQuote match
case x :: Nil => Literal(Constant(x))
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
val pos = call.sourcePos
CrossVersionChecks.checkExperimentalRef(call.symbol, pos)
withMode(Mode.InlinedCall)(transform(call))
val callTrace = Inlines.inlineCallTrace(call.symbol, pos)(using ctx.withSource(pos.source))
val callTrace = ref(call.symbol)(using ctx.withSource(pos.source)).withSpan(pos.span)
cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(using inlineContext(tree)))
case templ: Template =>
withNoCheckNews(templ.parents.flatMap(newPart)) {
Expand Down
12 changes: 6 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/YCheckPositions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class YCheckPositions extends Phase {
val currentSource = sources.head
assert(tree.source == currentSource, i"wrong source set for $tree # ${tree.uniqueId} of ${tree.getClass}, set to ${tree.source} but context had $currentSource\n ${tree.symbol.flagsString}")

// Recursivlely check children while keeping track of current source
// Recursively check children while keeping track of current source
reporting.trace(i"check pos ${tree.getClass} ${tree.source} ${sources.head} $tree") {
tree match {
case tree @ Inlined(_, bindings, expansion) if tree.inlinedFromOuterScope =>
Expand All @@ -46,7 +46,7 @@ class YCheckPositions extends Phase {
sources = old
case tree @ Inlined(call, bindings, expansion) =>
// bindings.foreach(traverse(_)) // TODO check inline proxies (see tests/tun/lst)
sources = call.symbol.topLevelClass.source :: sources
sources = call.symbol.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)(using inlineContext(tree).withSource(sources.head))
sources = sources.tail
Expand All @@ -61,10 +61,10 @@ class YCheckPositions extends Phase {

private def isMacro(call: Tree)(using Context) =
call.symbol.is(Macro) ||
(call.symbol.isClass && call.tpe.derivesFrom(defn.MacroAnnotationClass)) ||
// 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)
(!(ctx.phase <= postTyperPhase) && call.isInstanceOf[Select])
(call.symbol.isClass && call.tpe.derivesFrom(defn.MacroAnnotationClass)) ||
// In 3.0-3.3, the call of a macro after typer is encoded as a Select while other inlines are Ident.
// In those versions we kept the reference to the top-level class instead of the methods.
(!(ctx.phase <= postTyperPhase) && call.symbol.isClass && call.isInstanceOf[Select])

}

Expand Down

0 comments on commit f61026d

Please sign in to comment.