Add underlining to safe-init stack traces

Xavientois · Xavientois · commit c74bd98014bc · 2022-03-18T12:59:30.000-04:00
This underlines the source positions in the safe-init stack traces similarly to how the MessageRendering will underline its stack positions. Before, a safe-init error might look like this: ``` [error] -- Error: /******/dotty/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala:1032:22 [error] 1032 | "inlineVars" -> inlineVars [error] | ^^^^^^^^^^ [error] |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments [error] | [error] |The unsafe promotion may cause the following problem: [error] |Calling the external method method apply may cause initialization errors. Calling trace: [error] | -> "inlineVars" -> inlineVars [ PatternMatcher.scala:1032 ] [error] | -> Inliner(plan) [ PatternMatcher.scala:700 ] [error] | -> case plan: TestPlan => apply(plan) [ PatternMatcher.scala:492 ] [error] | -> plan.scrutinee = apply(plan.scrutinee) [ PatternMatcher.scala:472 ] [error] | -> def apply(tree: Tree): Tree = treeMap.transform(tree) [ PatternMatcher.scala:470 ] [error] | -> if (toDrop(sym)) transform(initializer(sym)) [ PatternMatcher.scala:675 ] ``` Now, the same error looks like this: ``` [error] -- Error: /******/dotty/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala:1032:22 [error] 1032 | "inlineVars" -> inlineVars [error] | ^^^^^^^^^^ [error] |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments [error] | [error] |The unsafe promotion may cause the following problem: [error] |Calling the external method method apply may cause initialization errors. Calling trace: [error] | -> "inlineVars" -> inlineVars [ PatternMatcher.scala:1032 ] [error] | ^^^^^^^^^^ [error] | -> Inliner(plan) [ PatternMatcher.scala:700 ] [error] | ^^^^^^^^^^^^^ [error] | -> case plan: TestPlan => apply(plan) [ PatternMatcher.scala:492 ] [error] | ^^^^^^^^^^^ [error] | -> plan.scrutinee = apply(plan.scrutinee) [ PatternMatcher.scala:472 ] [error] | ^^^^^^^^^^^^^^^^^^^^^ [error] | -> def apply(tree: Tree): Tree = treeMap.transform(tree) [ PatternMatcher.scala:470 ] [error] | ^^^^^^^^^^^^^^^^^^^^^^^ [error] | -> if (toDrop(sym)) transform(initializer(sym)) [ PatternMatcher.scala:675 ] ``` This should improve the readability of the safe-init error messages. Review by @liufengyun
diff --git a/compiler/src/dotty/tools/dotc/transform/init/Errors.scala b/compiler/src/dotty/tools/dotc/transform/init/Errors.scala
@@ -5,6 +5,7 @@ package init
 
 import ast.tpd._
 import core._
+import util.SourcePosition
 import Decorators._, printing.SyntaxHighlighting
 import Types._, Symbols._, Contexts._
 
@@ -40,14 +41,32 @@ object Errors {
             i"$code\t$loc"
           else
             tree.show
+        val positionMarkerLine =
+          if pos.exists && pos.source.exists then
+            positionMarker(pos, indentCount + 3)
+          else ""
 
-        if (last != line)  sb.append(prefix + line + "\n")
+        if (last != line)  sb.append(prefix + line + "\n" + positionMarkerLine )
 
         last = line
       }
       sb.toString
     }
 
+    /** Used to underline source positions in the stack trace
+     *  pos.source must exist
+     */
+    private def positionMarker(pos: SourcePosition, indent: Int): String = {
+      val trimmed = pos.lineContent.takeWhile(c => c.isWhitespace).length
+      val padding = pos.startColumnPadding.substring(trimmed).nn + (" " * indent)
+      val carets =
+        if (pos.startLine == pos.endLine)
+          "^" * math.max(1, pos.endColumn - pos.startColumn)
+        else "^"
+
+      s"$padding$carets\n"
+    }
+
     /** Flatten UnsafePromotion errors
      */
     def flatten: Errors = this match {
diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala
@@ -211,8 +211,8 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends
   def startColumnPadding(offset: Int): String = {
     var idx = startOfLine(offset)
     val pad = new StringBuilder
-    while (idx != offset) {
-      pad.append(if (idx < length && content()(idx) == '\t') '\t' else ' ')
+    while (idx != offset && idx < length && idx < content().length) {
+      pad.append(if (content()(idx) == '\t') '\t' else ' ')
       idx += 1
     }
     pad.result()
diff --git a/tests/init/neg/cycle-structure.check b/tests/init/neg/cycle-structure.check
@@ -3,12 +3,18 @@
   |             ^^^
   |             Access field A.this.b.x on a value with an unknown initialization status. Calling trace:
   |              -> val x = A(this)	[ cycle-structure.scala:9 ]
+  |                         ^^^^^^^
   |               -> case class A(b: B) {	[ cycle-structure.scala:1 ]
+  |                  ^
 -- Error: tests/init/neg/cycle-structure.scala:8:15 --------------------------------------------------------------------
 8 |    val x1 = a.x      // error
   |             ^^^
   |             Access field B.this.a.x on a value with an unknown initialization status. Calling trace:
   |              -> val x = A(this)	[ cycle-structure.scala:9 ]
+  |                         ^^^^^^^
   |               -> case class A(b: B) {	[ cycle-structure.scala:1 ]
+  |                  ^
   |                -> val x = B(this)	[ cycle-structure.scala:3 ]
+  |                           ^^^^^^^
   |                 -> case class B(a: A) {	[ cycle-structure.scala:7 ]
+  |                    ^
diff --git a/tests/init/neg/default-this.check b/tests/init/neg/default-this.check
@@ -3,3 +3,4 @@
   |        ^^^^^^^
   |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
   | -> val result = updateThenCompare(5)	[ default-this.scala:11 ]
+  |                 ^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init/neg/enum-desugared.check b/tests/init/neg/enum-desugared.check
@@ -6,7 +6,9 @@
    |          The unsafe promotion may cause the following problem:
    |          Calling the external method method name may cause initialization errors. Calling trace:
    |           -> Array(this.LazyErrorId, this.NoExplanationID) // error // error	[ enum-desugared.scala:17 ]
+   |                    ^^^^^^^^^^^^^^^^
    |            -> override def productPrefix: String = this.name()	[ enum-desugared.scala:29 ]
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 -- Error: tests/init/neg/enum-desugared.scala:17:33 --------------------------------------------------------------------
 17 |    Array(this.LazyErrorId, this.NoExplanationID) // error // error
    |                            ^^^^^^^^^^^^^^^^^^^^
@@ -15,4 +17,6 @@
    |           The unsafe promotion may cause the following problem:
    |           Calling the external method method ordinal may cause initialization errors. Calling trace:
    |            -> Array(this.LazyErrorId, this.NoExplanationID) // error // error	[ enum-desugared.scala:17 ]
+   |                                       ^^^^^^^^^^^^^^^^^^^^
    |             -> def errorNumber: Int = this.ordinal() - 2	[ enum-desugared.scala:8 ]
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init/neg/enum.check b/tests/init/neg/enum.check
@@ -6,3 +6,4 @@
   |        The unsafe promotion may cause the following problem:
   |        Calling the external method method name may cause initialization errors. Calling trace:
   |         -> NoExplanationID // error	[ enum.scala:4 ]
+  |            ^
diff --git a/tests/init/neg/inherit-non-hot.check b/tests/init/neg/inherit-non-hot.check
@@ -4,13 +4,21 @@
   |                        Cannot prove that the value is fully initialized. May only assign fully initialized value.
   |                         Calling trace:
   |                         -> val c = new C	[ inherit-non-hot.scala:19 ]
+  |                                    ^^^^^
   |                            -> class C extends A {	[ inherit-non-hot.scala:15 ]
+  |                               ^
   |                             -> val bAgain = toB.getBAgain	[ inherit-non-hot.scala:16 ]
+  |                                             ^^^
   |
   |                        The unsafe promotion may cause the following problem:
   |                        Call method Foo.B.this.aCopy.toB on a value with an unknown initialization. Calling trace:
   |                         -> val c = new C	[ inherit-non-hot.scala:19 ]
+  |                                    ^^^^^
   |                            -> class C extends A {	[ inherit-non-hot.scala:15 ]
+  |                               ^
   |                             -> val bAgain = toB.getBAgain	[ inherit-non-hot.scala:16 ]
+  |                                             ^^^
   |                              -> if b == null then b = new B(this) // error	[ inherit-non-hot.scala:6 ]
+  |                                                       ^^^^^^^^^^^
   |                               -> def getBAgain: B = aCopy.toB	[ inherit-non-hot.scala:12 ]
+  |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init/neg/inlined-method.check b/tests/init/neg/inlined-method.check
@@ -3,3 +3,4 @@
   |                                             ^^^^^^^
   |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
   | -> Assertion.failAssert(this)	[ inlined-method.scala:2 ]
+  |    ^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/init/neg/local-warm4.check b/tests/init/neg/local-warm4.check
@@ -3,9 +3,16 @@
    |                    ^^^^
    |       Cannot prove that the value is fully initialized. May only assign fully initialized value. Calling trace:
    |        -> val a = new A(5)	[ local-warm4.scala:26 ]
+   |                   ^^^^^^^^
    |           -> class A(x: Int) extends Foo(x) {	[ local-warm4.scala:6 ]
+   |              ^
    |            -> val b = new B(y)	[ local-warm4.scala:10 ]
+   |                       ^^^^^^^^
    |               -> class B(x: Int) extends A(x) {	[ local-warm4.scala:13 ]
+   |                  ^
    |                 -> class A(x: Int) extends Foo(x) {	[ local-warm4.scala:6 ]
+   |                    ^
    |                  -> increment()	[ local-warm4.scala:9 ]
+   |                     ^^^^^^^^^^^
    |                   -> updateA()	[ local-warm4.scala:21 ]
+   |                      ^^^^^^^^^
diff --git a/tests/init/neg/t3273.check b/tests/init/neg/t3273.check
@@ -6,6 +6,7 @@
   |             The unsafe promotion may cause the following problem:
   |             Access non-initialized value num1. Calling trace:
   |              -> val num1: LazyList[Int] = 1 #:: num1.map(_ + 1)                        // error	[ t3273.scala:4 ]
+  |                                                 ^^^^
 -- Error: tests/init/neg/t3273.scala:5:61 ------------------------------------------------------------------------------
 5 |  val num2: LazyList[Int] = 1 #:: num2.iterator.map(_ + 1).to(LazyList)  // error
   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -14,3 +15,4 @@
   |             The unsafe promotion may cause the following problem:
   |             Access non-initialized value num2. Calling trace:
   |              -> val num2: LazyList[Int] = 1 #:: num2.iterator.map(_ + 1).to(LazyList)  // error	[ t3273.scala:5 ]
+  |                                                 ^^^^
diff --git a/tests/init/neg/unsound2.check b/tests/init/neg/unsound2.check
@@ -3,4 +3,6 @@
   |                        ^^^
   | Access field B.this.a.n on a value with an unknown initialization status. Calling trace:
   |  -> println(foo(x).getB)	[ unsound2.scala:8 ]
+  |             ^^^^^^
   |   -> def foo(y: Int): B = if (y > 10) then B(bar(y - 1), foo(y - 1).getN) else B(bar(y), 10)	[ unsound2.scala:2 ]
+  |                                                          ^^^^^^^^^^^^^^^
diff --git a/tests/init/neg/unsound3.check b/tests/init/neg/unsound3.check
@@ -3,3 +3,4 @@
    |                         ^^^^^^^^^^^^^^
    |             Access field C.this.foo().getC().b on a value with an unknown initialization status. Calling trace:
    |              -> val b = foo()	[ unsound3.scala:12 ]
+   |                         ^^^^^
diff --git a/tests/init/neg/unsound4.check b/tests/init/neg/unsound4.check
@@ -3,4 +3,6 @@
   |        ^
   |        Access non-initialized value aAgain. Calling trace:
   |         -> val aAgain = foo(5) // error	[ unsound4.scala:3 ]
+  |                         ^^^^^^
   |          -> def foo(x: Int): A = if (x < 5) then this else foo(x - 1).aAgain	[ unsound4.scala:2 ]
+  |                                                            ^^^^^^^^^^^^^^^^^
