Skip to content

Only track inlined positions #9966

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Only track inlined positions #9966

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
767e24e
Only track inlined positions
nicolasstucki Oct 7, 2020
c59e7aa
Simplify tracking of inlined positions
nicolasstucki Oct 8, 2020
18a8d51
Track inlined positions from tree instead of call
nicolasstucki Oct 8, 2020
f0e4189
Remove `call` of `Inlined` after `PostTyper`
nicolasstucki Oct 8, 2020
7614be6
Use SourcePosition directly
nicolasstucki Oct 8, 2020
48c88b5
Update compiler/src/dotty/tools/dotc/ast/Trees.scala
nicolasstucki Oct 12, 2020
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
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ 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 YCheckPositions) :: // YCheck positions
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we have to remove the phase? Does that mean positions are not checked anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YCheckPositions used to check that the sources we have on the trees align with the source that is in the call of the Inlined trees. Initially, this was used when we added sources to the trees to make sure those where correct and catch Inlined trees that had an empty/non-nonempty call when they should not. This was important to make sure the positions of the trees where properly unpickled and that the transformation phases did not lose information about the sources on the Inlined.calls before erasure.
This invariant does not hold after PostTyper anymore as all calls are empty and all the inlined source logic relies on the sources of the trees instead of the calls. Therefore we can't check what we were originally checking and we don't need to as we use exclusively the sources of the trees.

List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
List(new semanticdb.ExtractSemanticDB) :: // Extract info into .semanticdb files
List(new PostTyper) :: // Additional checks and cleanups after type checking
Expand Down
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ object PickledQuotes {
def pickleQuote(tree: Tree)(using Context): PickledQuote =
if (ctx.reporter.hasErrors) Nil
else {
assert(!tree.isInstanceOf[Hole]) // Should not be pickled as it represents `'{$x}` which should be optimized to `x`
// FIXME handle new optimization opportunities. Previousliy we got some `Inlined(EmptyTree, Nil, Hole(...))` that did not get optimized
// assert(!tree.isInstanceOf[Hole]) // Should not be pickled as it represents `'{$x}` which should be optimized to `x`
val pickled = pickle(tree)
TastyString.pickle(pickled)
}
Expand All @@ -56,11 +57,9 @@ object PickledQuotes {
val tastyBytes = TastyString.unpickle(tasty)
val unpickled = withMode(Mode.ReadPositions)(
unpickle(tastyBytes, splices, isType = false))
val Inlined(call, Nil, expnasion) = unpickled
val inlineCtx = inlineContext(call)
val expansion1 = spliceTypes(expnasion, splices)(using inlineCtx)
val expansion2 = spliceTerms(expansion1, splices)(using inlineCtx)
cpy.Inlined(unpickled)(call, Nil, expansion2)
val expansion1 = spliceTypes(unpickled, splices)
val expansion2 = spliceTerms(expansion1, splices)
cpy.Inlined(unpickled)(EmptyTree, Nil, expansion2)
}

/** Unpickle the tree contained in the TastyType */
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/transform/PostTyper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
super.transform(tree1)
}
case Inlined(call, bindings, expansion) if !call.isEmpty =>
val pos = call.sourcePos
val callTrace = Inliner.inlineCallTrace(call.symbol, pos)(using ctx.withSource(pos.source))
cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion)(using inlineContext(tree)))
cpy.Inlined(tree)(EmptyTree, transformSub(bindings), transform(expansion)(using inlineContext(tree)))
case templ: Template =>
withNoCheckNews(templ.parents.flatMap(newPart)) {
forwardParamAccessors(templ)
Expand Down
10 changes: 2 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/ReifyQuotes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,8 @@ class ReifyQuotes extends MacroTransform {
}
else {
val (body1, splices) = nested(isQuote = true).splitQuote(body)(using quoteContext)
if (level == 0) {
val body2 =
if (body1.isType) body1
else Inlined(Inliner.inlineCallTrace(ctx.owner, quote.sourcePos), Nil, body1)
pickledQuote(body2, splices, body.tpe, isType).withSpan(quote.span)
}
else
body
if level == 0 then pickledQuote(body1, splices, body.tpe, isType).withSpan(quote.span)
else body
}
}

Expand Down
62 changes: 0 additions & 62 deletions compiler/src/dotty/tools/dotc/transform/YCheckPositions.scala
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions compiler/src/dotty/tools/dotc/typer/Inliner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,19 +258,6 @@ object Inliner {
(new Reposition).transform(tree)
}

/** 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)
if (callSym.is(Macro)) ref(callSym.topLevelClass.owner).select(callSym.topLevelClass.name).withSpan(pos.span)
else Ident(callSym.topLevelClass.typeRef).withSpan(pos.span)
}

object Intrinsics {
import dotty.tools.dotc.reporting.Diagnostic.Error
private enum ErrorKind:
Expand Down