Skip to content

Commit 81ef061

Browse files
committed
Collapse positions in macro expansions after duplication
Macro expansion changes positions in the expanded tree to offset (in `validateResultingTree`). For spliced, non-duplicated trees this can modify the positions of the expandee, which means that the `MacroExpansionAttachment` attachment doesn't contain the original tree with range positions. This PR collapses positions of macro expansions after they are duplicated. The duplication is already in place to avoid structural sharing in macro-generated trees, a "mistake" that's easy to do by macro authors.
1 parent d950286 commit 81ef061

File tree

1 file changed

+28
-25
lines changed

1 file changed

+28
-25
lines changed

src/compiler/scala/tools/nsc/typechecker/Macros.scala

+28-25
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,31 @@ trait Macros extends MacroRuntimes with Traces with Helpers {
610610
}
611611

612612
protected def expand(desugared: Tree): Tree = {
613+
def positionsToOffset(expanded: Tree) = {
614+
// Macros might have spliced arguments with range positions into non-compliant
615+
// locations, notably, under a tree without a range position. Or, they might
616+
// splice a tree that `resetAttrs` has assigned NoPosition.
617+
//
618+
// Here, we just convert all positions in the tree to offset positions, and
619+
// convert NoPositions to something sensible.
620+
//
621+
// Given that the IDE now sees the expandee (by using -Ymacro-expand:discard),
622+
// this loss of position fidelity shouldn't cause any real problems.
623+
//
624+
// Alternatively, we could pursue a way to exclude macro expansions from position
625+
// invariant checking, or find a way not to touch expansions that happen to validate.
626+
//
627+
// This would be useful for cases like:
628+
//
629+
// macro1 { macro2 { "foo" } }
630+
//
631+
// to allow `macro1` to see the range position of the "foo".
632+
val expandedPos = enclosingMacroPosition.focus
633+
def fixPosition(pos: Position) =
634+
if (pos == NoPosition) expandedPos else pos.focus
635+
expanded.foreach(t => t.pos = fixPosition(t.pos))
636+
expanded
637+
}
613638
def showDetailed(tree: Tree) = showRaw(tree, printIds = true, printTypes = true)
614639
def summary() = s"expander = $this, expandee = ${showDetailed(expandee)}, desugared = ${if (expandee == desugared) () else showDetailed(desugared)}"
615640
if (macroDebugVerbose) println(s"macroExpand: ${summary()}")
@@ -631,8 +656,9 @@ trait Macros extends MacroRuntimes with Traces with Helpers {
631656
}
632657
expanded match {
633658
case Success(expanded) =>
634-
// also see http://groups.google.com/group/scala-internals/browse_thread/thread/492560d941b315cc
635-
val expanded1 = try onSuccess(duplicateAndKeepPositions(expanded)) finally popMacroContext()
659+
// duplicate expanded tree to avoid structural sharing in macro-genrated trees
660+
// see http://groups.google.com/group/scala-internals/browse_thread/thread/492560d941b315cc
661+
val expanded1 = try onSuccess(positionsToOffset(duplicateAndKeepPositions(expanded))) finally popMacroContext()
636662
if (!hasMacroExpansionAttachment(expanded1)) linkExpandeeAndExpanded(expandee, expanded1)
637663
if (settings.Ymacroexpand.value == settings.MacroExpand.Discard && !typer.context.isSearchingForImplicitParam) {
638664
suppressMacroExpansion(expandee)
@@ -827,29 +853,6 @@ trait Macros extends MacroRuntimes with Traces with Helpers {
827853
macroLogLite("" + expanded + "\n" + showRaw(expanded))
828854
val freeSyms = expanded.freeSyms
829855
freeSyms foreach (sym => MacroFreeSymbolError(expandee, sym))
830-
// Macros might have spliced arguments with range positions into non-compliant
831-
// locations, notably, under a tree without a range position. Or, they might
832-
// splice a tree that `resetAttrs` has assigned NoPosition.
833-
//
834-
// Here, we just convert all positions in the tree to offset positions, and
835-
// convert NoPositions to something sensible.
836-
//
837-
// Given that the IDE now sees the expandee (by using -Ymacro-expand:discard),
838-
// this loss of position fidelity shouldn't cause any real problems.
839-
//
840-
// Alternatively, we could pursue a way to exclude macro expansions from position
841-
// invariant checking, or find a way not to touch expansions that happen to validate.
842-
//
843-
// This would be useful for cases like:
844-
//
845-
// macro1 { macro2 { "foo" } }
846-
//
847-
// to allow `macro1` to see the range position of the "foo".
848-
val expandedPos = enclosingMacroPosition.focus
849-
def fixPosition(pos: Position) =
850-
if (pos == NoPosition) expandedPos else pos.focus
851-
expanded.foreach(t => t.pos = fixPosition(t.pos))
852-
853856
val result = atPos(enclosingMacroPosition.focus)(expanded)
854857
Success(result)
855858
}

0 commit comments

Comments
 (0)