Skip to content

Commit 6c18e37

Browse files
committed
Address reviewer comments.
1 parent 13e3d59 commit 6c18e37

File tree

8 files changed

+46
-40
lines changed

8 files changed

+46
-40
lines changed

src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,15 +500,15 @@ object desugar {
500500
* val/var/lazy val p = e
501501
*
502502
* Otherwise, in case there is exactly one variable x_1 in pattern
503-
* val/var/lazy val p = e ==> val/var x_1 = (e: @unchecked) match (case p => (x_1))
503+
* val/var/lazy val p = e ==> val/var/lazy val x_1 = (e: @unchecked) match (case p => (x_1))
504504
*
505505
* in case there are zero or more than one variables in pattern
506506
* val/var/lazy p = e ==> private synthetic [lazy] val t$ = (e: @unchecked) match (case p => (x_1, ..., x_N))
507507
* val/var/def x_1 = t$._1
508508
* ...
509509
* val/var/def x_N = t$._N
510510
* If the original pattern variable carries a type annotation, so does the corresponding
511-
* ValDef.
511+
* ValDef or DefDef.
512512
*/
513513
def makePatDef(mods: Modifiers, pat: Tree, rhs: Tree)(implicit ctx: Context): Tree = pat match {
514514
case VarPattern(named, tpt) =>

src/dotty/tools/dotc/ast/NavigateAST.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ object NavigateAST {
4747
}
4848
case _ =>
4949
untypedPath(tree.pos) match {
50-
case path @ last :: _ if last.pos == tree.pos || !exactMatch => path
50+
case (path @ last :: _) if last.pos == tree.pos || !exactMatch => path
5151
case _ => Nil
5252
}
5353
}

src/dotty/tools/dotc/ast/Positioned.scala

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,35 +54,44 @@ abstract class Positioned extends DotClass with Product {
5454
*/
5555
private[dotc] def setPosUnchecked(pos: Position) = curPos = pos
5656

57-
/** If any children of this node do not have positions, set them to the given position,
57+
/** If any children of this node do not have positions,
58+
* fit their positions between the positions of the known subtrees
5859
* and transitively visit their children.
60+
* The method is likely time-critical because it is invoked on any node
61+
* we create, so we want to avoid object allocations in the common case.
62+
* The method is naturally expressed as two mutually (tail-)recursive
63+
* functions, one which computes the next element to consider or terminates if there
64+
* is none and the other which propagates the position information to that element.
65+
* But since mutual tail recursion is not supported in Scala, we express it instead
66+
* as a while loop with a termination by return in the middle.
5967
*/
6068
private def setChildPositions(pos: Position): Unit = {
61-
var n = productArity
62-
var elems: List[Any] = Nil
63-
var end = pos.end
64-
var outstanding: List[Positioned] = Nil
69+
var n = productArity // subnodes are analyzed right to left
70+
var elems: List[Any] = Nil // children in lists still to be considered, from right to left
71+
var end = pos.end // the last defined offset, fill in positions up to this offset
72+
var outstanding: List[Positioned] = Nil // nodes that need their positions filled once a start position
73+
// is known, from left to right.
6574
def fillIn(ps: List[Positioned], start: Int, end: Int): Unit = ps match {
6675
case p :: ps1 =>
6776
p.setPos(Position(start, end))
6877
fillIn(ps1, end, end)
6978
case nil =>
7079
}
7180
while (true) {
72-
var nextElem: Any = null
81+
var nextChild: Any = null // the next child to be considered
7382
if (elems.nonEmpty) {
74-
nextElem = elems.head
83+
nextChild = elems.head
7584
elems = elems.tail
7685
}
7786
else if (n > 0) {
7887
n = n - 1
79-
nextElem = productElement(n)
88+
nextChild = productElement(n)
8089
}
8190
else {
8291
fillIn(outstanding, pos.start, end)
8392
return
8493
}
85-
nextElem match {
94+
nextChild match {
8695
case p: Positioned =>
8796
if (p.pos.exists) {
8897
fillIn(outstanding, p.pos.end, end)
@@ -91,8 +100,7 @@ abstract class Positioned extends DotClass with Product {
91100
}
92101
else outstanding = p :: outstanding
93102
case xs: List[_] =>
94-
val newElems = xs.reverse
95-
elems = if (elems.isEmpty) newElems else newElems ::: elems
103+
elems = elems ::: xs.reverse
96104
case _ =>
97105
}
98106
}

src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1763,7 +1763,9 @@ object Parsers {
17631763
*/
17641764
def defDefOrDcl(mods: Modifiers): Tree = atPos(tokenRange) {
17651765
def scala2ProcedureSyntax(resultTypeStr: String) = {
1766-
val toInsert = if (in.token == LBRACE) s"$resultTypeStr =" else ": Unit "
1766+
val toInsert =
1767+
if (in.token == LBRACE) s"$resultTypeStr ="
1768+
else ": Unit " // trailing space ensures that `def f()def g()` works.
17671769
testScala2Mode(s"Procedure syntax no longer supported; `$toInsert' should be inserted here") && {
17681770
patch(source, Position(in.lastOffset), toInsert)
17691771
true

src/dotty/tools/dotc/rewrite/Rewrites.scala

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ object Rewrites {
5151

5252
def writeBack(): Unit = {
5353
val out = source.file.output
54-
val chars = apply(source.content)
54+
val chars = apply(source.underlying.content)
5555
val bytes = new String(chars).getBytes
5656
out.write(bytes)
5757
out.close()
@@ -62,28 +62,21 @@ object Rewrites {
6262
* given by `pos` in `source` by `replacement`
6363
*/
6464
def patch(source: SourceFile, pos: Position, replacement: String)(implicit ctx: Context): Unit =
65-
ctx.settings.rewrite.value match {
66-
case Some(rewrites: Rewrites) =>
67-
rewrites.patched.get(source) match {
68-
case Some(ps) =>
69-
ps.addPatch(pos, replacement)
70-
case None =>
71-
rewrites.patched(source) = new Patches(source)
72-
patch(source, pos, replacement)
73-
}
74-
case _ =>
75-
}
65+
for (rewrites <- ctx.settings.rewrite.value)
66+
rewrites.patched
67+
.getOrElseUpdate(source, new Patches(source))
68+
.addPatch(pos, replacement)
69+
70+
/** Patch position in `ctx.compilationUnit.source`. */
71+
def patch(pos: Position, replacement: String)(implicit ctx: Context): Unit =
72+
patch(ctx.compilationUnit.source, pos, replacement)
7673

7774
/** If -rewrite is set, apply all patches and overwrite patched source files.
7875
*/
7976
def writeBack()(implicit ctx: Context) =
80-
ctx.settings.rewrite.value match {
81-
case Some(rewrites: Rewrites) =>
82-
for (source <- rewrites.patched.keys) {
83-
ctx.println(s"[patched file ${source.file.path}]")
84-
rewrites.patched(source).writeBack()
85-
}
86-
case _ =>
77+
for (rewrites <- ctx.settings.rewrite.value; source <- rewrites.patched.keys) {
78+
ctx.println(s"[patched file ${source.file.path}]")
79+
rewrites.patched(source).writeBack()
8780
}
8881
}
8982

src/dotty/tools/dotc/transform/LazyVals.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with Nee
7070
val isField = sym.owner.isClass
7171
if (isField) {
7272
if (sym.isVolatile ||
73-
(sym.is(Flags.Module)/* || ctx.scala2Mode*/) && !sym.is(Flags.Synthetic)) // TODO assume @voliat
73+
(sym.is(Flags.Module)/* || ctx.scala2Mode*/) &&
74+
// TODO assume @volatile once LazyVals uses static helper constructs instead of
75+
// ones in the companion object.
76+
!sym.is(Flags.Synthetic))
7477
// module class is user-defined.
7578
// Should be threadsafe, to mimic safety guaranteed by global object
7679
transformMemberDefVolatile(tree)

src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
998998
if (sym.is(Lazy, butNot = Deferred | Module | Synthetic) && !sym.isVolatile &&
999999
ctx.scala2Mode && ctx.settings.rewrite.value.isDefined &&
10001000
!ctx.isAfterTyper)
1001-
patch(ctx.compilationUnit.source, Position(toUntyped(vdef).envelope.start), "@volatile ")
1001+
patch(Position(toUntyped(vdef).envelope.start), "@volatile ")
10021002
}
10031003

10041004
def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context) = track("typedDefDef") {
@@ -1158,8 +1158,8 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
11581158
if (ctx.scala2Mode) {
11591159
// Under -rewrite, patch `x _` to `(() => x)`
11601160
ctx.migrationWarning(msg, tree.pos)
1161-
patch(ctx.compilationUnit.source, Position(tree.pos.start), "(() => ")
1162-
patch(ctx.compilationUnit.source, Position(qual.pos.end, tree.pos.end), ")")
1161+
patch(Position(tree.pos.start), "(() => ")
1162+
patch(Position(qual.pos.end, tree.pos.end), ")")
11631163
res = typed(untpd.Function(Nil, untpd.TypedSplice(res)))
11641164
}
11651165
else ctx.error(msg, tree.pos)

src/dotty/tools/dotc/typer/VarianceChecker.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ class VarianceChecker()(implicit ctx: Context) {
114114
case Some(VarianceError(tvar, required)) =>
115115
def msg = i"${varianceString(tvar.flags)} $tvar occurs in ${varianceString(required)} position in type ${sym.info} of $sym"
116116
if (ctx.scala2Mode && sym.owner.isConstructor) {
117-
ctx.migrationWarning(s"According to new variance rules, this is no longer accepted; need to annotate with @uncheckedVariance:\n$msg, pos = ${sym.pos}", sym.pos)
118-
patch(ctx.compilationUnit.source, Position(pos.end), " @scala.annotation.unchecked.uncheckedVariance") // TODO use an import or shorten if possible
117+
ctx.migrationWarning(s"According to new variance rules, this is no longer accepted; need to annotate with @uncheckedVariance:\n$msg", sym.pos)
118+
patch(Position(pos.end), " @scala.annotation.unchecked.uncheckedVariance") // TODO use an import or shorten if possible
119119
}
120120
else ctx.error(msg, sym.pos)
121121
case None =>

0 commit comments

Comments
 (0)