Skip to content
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.

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

Show inlined positions with source code #14427

Merged
merged 4 commits into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
6 changes: 4 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ jobs:
- name: Cmd Tests
run: |
./project/scripts/sbt ";dist/pack; scala3-bootstrapped/compile; scala3-bootstrapped/test;sjsSandbox/run;sjsSandbox/test;sjsJUnitTests/test;sjsCompilerTests/test ;sbt-test/scripted scala2-compat/* ;configureIDE ;stdlib-bootstrapped/test:run ;stdlib-bootstrapped-tasty-tests/test; scala3-compiler-bootstrapped/scala3CompilerCoursierTest:test"
./project/scripts/bootstrapCmdTests
./project/scripts/cmdTests
./project/scripts/bootstrappedOnlyCmdTests

- name: MiMa
run: |
Expand Down Expand Up @@ -447,7 +448,8 @@ jobs:
- name: Test
run: |
./project/scripts/sbt ";dist/pack ;scala3-bootstrapped/compile ;scala3-bootstrapped/test;sjsSandbox/run;sjsSandbox/test;sjsJUnitTests/test;sjsCompilerTests/test ;sbt-test/scripted scala2-compat/* ;configureIDE ;stdlib-bootstrapped/test:run ;stdlib-bootstrapped-tasty-tests/test"
./project/scripts/bootstrapCmdTests
./project/scripts/cmdTests
./project/scripts/bootstrappedOnlyCmdTests

publish_nightly:
runs-on: [self-hosted, Linux]
Expand Down
184 changes: 136 additions & 48 deletions compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import scala.annotation.switch
import scala.collection.mutable

trait MessageRendering {
import Highlight.*
import Offsets.*

/** Remove ANSI coloring from `str`, useful for getting real length of
* strings
Expand All @@ -25,31 +27,25 @@ trait MessageRendering {
def stripColor(str: String): String =
str.replaceAll("\u001b\\[.*?m", "")

/** When inlining a method call, if there's an error we'd like to get the
* outer context and the `pos` at which the call was inlined.
*
* @return a list of strings with inline locations
*/
def outer(pos: SourcePosition, prefix: String)(using Context): List[String] =
if (pos.outer.exists)
i"$prefix| This location contains code that was inlined from $pos" ::
outer(pos.outer, prefix)
/** List of all the inline calls that surround the position */
def inlinePosStack(pos: SourcePosition): List[SourcePosition] =
if pos.outer != null && pos.outer.exists then pos :: inlinePosStack(pos.outer)
else Nil

/** Get the sourcelines before and after the position, as well as the offset
* for rendering line numbers
*
* @return (lines before error, lines after error, line numbers offset)
*/
def sourceLines(pos: SourcePosition, diagnosticLevel: String)(using Context): (List[String], List[String], Int) = {
private def sourceLines(pos: SourcePosition)(using Context, Level, Offset): (List[String], List[String], Int) = {
assert(pos.exists && pos.source.file.exists)
var maxLen = Int.MinValue
def render(offsetAndLine: (Int, String)): String = {
val (offset, line) = offsetAndLine
val lineNbr = pos.source.offsetToLine(offset)
val prefix = s"${lineNbr + 1} |"
val (offset1, line) = offsetAndLine
val lineNbr = (pos.source.offsetToLine(offset1) + 1).toString
val prefix = String.format(s"%${offset - 2}s |", lineNbr)
maxLen = math.max(maxLen, prefix.length)
val lnum = hl(diagnosticLevel)(" " * math.max(0, maxLen - prefix.length) + prefix)
val lnum = hl(" " * math.max(0, maxLen - prefix.length - 1) + prefix)
lnum + line.stripLineEnd
}

Expand Down Expand Up @@ -77,23 +73,75 @@ trait MessageRendering {
)
}

/** The column markers aligned under the error */
def columnMarker(pos: SourcePosition, offset: Int, diagnosticLevel: String)(using Context): String = {
val prefix = " " * (offset - 1)
/** Generate box containing the report title
*
* ```
* -- Error: source.scala ---------------------
* ```
*/
private def boxTitle(title: String)(using Context, Level, Offset): String =
val pageWidth = ctx.settings.pageWidth.value
val line = "-" * (pageWidth - title.length - 4)
hl(s"-- $title $line")

/** The position markers aligned under the error
*
* ```
* | ^^^^^
* ```
*/
private def positionMarker(pos: SourcePosition)(using Context, Level, Offset): String = {
val padding = pos.startColumnPadding
val carets = hl(diagnosticLevel) {
val carets =
if (pos.startLine == pos.endLine)
"^" * math.max(1, pos.endColumn - pos.startColumn)
else "^"
}
s"$prefix|$padding$carets"
hl(s"$offsetBox$padding$carets")
}

/** The horizontal line with the given offset
*
* ```
* |
* ```
*/
private def offsetBox(using Context, Level, Offset): String =
val prefix = " " * (offset - 1)
hl(s"$prefix|")

/** The end of a box section
*
* ```
* |---------------
* ```
* Or if there `soft` is true,
* ```
* |···············
* ```
*/
private def newBox(soft: Boolean = false)(using Context, Level, Offset): String =
val pageWidth = ctx.settings.pageWidth.value
val prefix = " " * (offset - 1)
val line = (if soft then "·" else "-") * (pageWidth - offset)
hl(s"$prefix|$line")

/** The end of a box section
*
* ```
* ·----------------
* ```
*/
private def endBox(using Context, Level, Offset): String =
val pageWidth = ctx.settings.pageWidth.value
val prefix = " " * (offset - 1)
val line = "-" * (pageWidth - offset)
hl(s"${prefix}·$line")

/** The error message (`msg`) aligned under `pos`
*
* @return aligned error message
*/
def errorMsg(pos: SourcePosition, msg: String, offset: Int)(using Context): String = {
private def errorMsg(pos: SourcePosition, msg: String)(using Context, Level, Offset): String = {
val padding = msg.linesIterator.foldLeft(pos.startColumnPadding) { (pad, line) =>
val lineLength = stripColor(line).length
val maxPad = math.max(0, ctx.settings.pageWidth.value - offset - lineLength) - offset
Expand All @@ -103,35 +151,35 @@ trait MessageRendering {
}

msg.linesIterator
.map { line => " " * (offset - 1) + "|" + (if line.isEmpty then "" else padding + line) }
.map { line => offsetBox + (if line.isEmpty then "" else padding + line) }
.mkString(EOL)
}

/** The source file path, line and column numbers from the given SourcePosition */
def posFileStr(pos: SourcePosition): String =
protected def posFileStr(pos: SourcePosition): String =
val path = pos.source.file.path
if pos.exists then s"$path:${pos.line + 1}:${pos.column}" else path

/** The separator between errors containing the source file and error type
*
* @return separator containing error location and kind
*/
def posStr(pos: SourcePosition, diagnosticLevel: String, message: Message)(using Context): String =
if (pos.source != NoSourcePosition.source) hl(diagnosticLevel)({
val fileAndPos = posFileStr(pos.nonInlined)
val file = if fileAndPos.isEmpty || fileAndPos.endsWith(" ") then fileAndPos else s"$fileAndPos "
private def posStr(pos: SourcePosition, message: Message, diagnosticString: String)(using Context, Level, Offset): String =
if (pos.source != NoSourcePosition.source) hl({
val realPos = pos.nonInlined
val fileAndPos = posFileStr(realPos)
val errId =
if (message.errorId ne ErrorMessageID.NoExplanationID) {
val errorNumber = message.errorId.errorNumber
s"[E${"0" * (3 - errorNumber.toString.length) + errorNumber}] "
} else ""
val kind =
if (message.kind == "") diagnosticLevel
else s"${message.kind} $diagnosticLevel"
val prefix = s"-- ${errId}${kind}: $file"

prefix +
("-" * math.max(ctx.settings.pageWidth.value - stripColor(prefix).length, 0))
if (message.kind == "") diagnosticString
else s"${message.kind} $diagnosticString"
val title =
if fileAndPos.isEmpty then s"$errId$kind:" // this happens in dotty.tools.repl.ScriptedTests // TODO add name of source or remove `:` (and update test files)
else s"$errId$kind: $fileAndPos"
boxTitle(title)
}) else ""

/** Explanation rendered under "Explanation" header */
Expand All @@ -146,7 +194,7 @@ trait MessageRendering {
sb.toString
}

def appendFilterHelp(dia: Diagnostic, sb: mutable.StringBuilder): Unit =
private def appendFilterHelp(dia: Diagnostic, sb: mutable.StringBuilder): Unit =
import dia._
val hasId = msg.errorId.errorNumber >= 0
val category = dia match {
Expand All @@ -166,17 +214,35 @@ trait MessageRendering {
/** The whole message rendered from `msg` */
def messageAndPos(dia: Diagnostic)(using Context): String = {
import dia._
val levelString = diagnosticLevel(dia)
val pos1 = pos.nonInlined
val inlineStack = inlinePosStack(pos).filter(_ != pos1)
val maxLineNumber =
if pos.exists then (pos1 :: inlineStack).map(_.endLine).max + 1
else 0
given Level = Level(level)
given Offset = Offset(maxLineNumber.toString.length + 2)
val sb = mutable.StringBuilder()
val posString = posStr(pos, levelString, msg)
val posString = posStr(pos, msg, diagnosticLevel(dia))
if (posString.nonEmpty) sb.append(posString).append(EOL)
if (pos.exists) {
val pos1 = pos.nonInlined
if (pos1.exists && pos1.source.file.exists) {
val (srcBefore, srcAfter, offset) = sourceLines(pos1, levelString)
val marker = columnMarker(pos1, offset, levelString)
val err = errorMsg(pos1, msg.message, offset)
sb.append((srcBefore ::: marker :: err :: outer(pos, " " * (offset - 1)) ::: srcAfter).mkString(EOL))
val (srcBefore, srcAfter, offset) = sourceLines(pos1)
val marker = positionMarker(pos1)
val err = errorMsg(pos1, msg.message)
sb.append((srcBefore ::: marker :: err :: srcAfter).mkString(EOL))

if inlineStack.nonEmpty then
sb.append(EOL).append(newBox())
sb.append(EOL).append(offsetBox).append(i"Inline stack trace")
for inlinedPos <- inlineStack if inlinedPos != pos1 do
sb.append(EOL).append(newBox(soft = true))
sb.append(EOL).append(offsetBox).append(i"This location contains code that was inlined from $pos")
if inlinedPos.source.file.exists then
val (srcBefore, srcAfter, _) = sourceLines(inlinedPos)
val marker = positionMarker(inlinedPos)
sb.append(EOL).append((srcBefore ::: marker :: srcAfter).mkString(EOL))
sb.append(EOL).append(endBox)
}
else sb.append(msg.message)
}
Expand All @@ -186,15 +252,13 @@ trait MessageRendering {
sb.toString
}

def hl(diagnosticLevel: String)(str: String)(using Context): String = diagnosticLevel match {
case "Info" => Blue(str).show
case "Error" => Red(str).show
case _ =>
assert(diagnosticLevel.contains("Warning"))
Yellow(str).show
}
private def hl(str: String)(using Context, Level): String =
summon[Level].value match
case interfaces.Diagnostic.ERROR => Red(str).show
case interfaces.Diagnostic.WARNING => Yellow(str).show
case interfaces.Diagnostic.INFO => Blue(str).show

def diagnosticLevel(dia: Diagnostic): String =
private def diagnosticLevel(dia: Diagnostic): String =
dia match {
case dia: FeatureWarning => "Feature Warning"
case dia: DeprecationWarning => "Deprecation Warning"
Expand All @@ -205,4 +269,28 @@ trait MessageRendering {
case interfaces.Diagnostic.WARNING => "Warning"
case interfaces.Diagnostic.INFO => "Info"
}

}

private object Highlight {
opaque type Level = Int
extension (level: Level) def value: Int = level
object Level:
def apply(level: Int): Level = level
}

/** Size of the left offset added by the box
*
* ```
* -- Error: ... ------------
* 4 | foo
* | ^^^
* ^^^ // size of this offset
* ```
*/
private object Offsets {
opaque type Offset = Int
def offset(using o: Offset): Int = o
object Offset:
def apply(level: Int): Offset = level
}
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Splicer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ object Splicer {
val oldContextClassLoader = Thread.currentThread().getContextClassLoader
Thread.currentThread().setContextClassLoader(classLoader)
try {
val interpreter = new Interpreter(spliceExpansionPos, classLoader)
val interpreter = new Interpreter(splicePos, classLoader)

// Some parts of the macro are evaluated during the unpickling performed in quotedExprToTree
val interpretedExpr = interpreter.interpret[Quotes => scala.quoted.Expr[Any]](tree)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
evidence.tpe match
case fail: Implicits.SearchFailureType =>
val msg = evTyper.missingArgMsg(evidence, tpt.tpe, "")
errorTree(tpt, em"$msg")
errorTree(call, em"$msg")
case _ =>
evidence
return searchImplicit(callTypeArgs.head)
Expand Down
1 change: 0 additions & 1 deletion compiler/test-resources/repl/i9227
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@ scala> import scala.quoted._; inline def myMacro[T]: Unit = ${ myMacroImpl[T] };
1 | import scala.quoted._; inline def myMacro[T]: Unit = ${ myMacroImpl[T] }; def myMacroImpl[T](using Quotes): Expr[Unit] = '{}; println(myMacro[Int])
| ^^^^^^^^^^^^
| Cannot call macro method myMacroImpl defined in the same source file
| This location contains code that was inlined from rs$line$1:1
1 error found
1 change: 1 addition & 0 deletions project/scripts/cmdTests
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ cp tests/neg/i6371/B_2.scala $OUT/B.scala
"$SBT" "scalac $OUT/A.scala -d $OUT1"
rm $OUT/A.scala
"$SBT" "scalac -classpath $OUT1 -d $OUT1 $OUT/B.scala" > "$tmp" 2>&1 || echo "ok"
cat "$tmp" # for debugging
Copy link
Member

Choose a reason for hiding this comment

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

Could you comment that out or only display it in case the test doesn't pass? Otherwise we get some error output in the CI that could be misinterpreted as a test failure.

Copy link
Contributor Author

@nicolasstucki nicolasstucki Feb 8, 2022

Choose a reason for hiding this comment

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

Commented out

grep -qe "B.scala:2:7" "$tmp"
grep -qe "This location contains code that was inlined from A.scala:3" "$tmp"

Expand Down
1 change: 0 additions & 1 deletion tests/neg-macros/delegate-match-1.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
| ^
| AmbiguousImplicits
| both value a1 in class Test1 and value a2 in class Test1 match type A
| This location contains code that was inlined from Test_2.scala:6
1 change: 0 additions & 1 deletion tests/neg-macros/delegate-match-2.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
| ^
| DivergingImplicit
| method a1 in class Test produces a diverging implicit search when trying to match type A
| This location contains code that was inlined from Test_2.scala:5
1 change: 0 additions & 1 deletion tests/neg-macros/delegate-match-3.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
| ^
| NoMatchingImplicits
| no implicit values were found that match type A
| This location contains code that was inlined from Test_2.scala:3
18 changes: 14 additions & 4 deletions tests/neg-macros/i11386.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,21 @@
6 | dummy(0) // error
| ^
| test
| This location contains code that was inlined from Test_2.scala:6
| This location contains code that was inlined from Macro_1.scala:7
|---------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|·····················································································································
|This location contains code that was inlined from Test_2.scala:6
7 | notNull(i)
| ^^^^^^^^^^
·---------------------------------------------------------------------------------------------------------------------
-- Error: tests/neg-macros/i11386/Test_2.scala:8:20 --------------------------------------------------------------------
8 | dummy(int2String(0)) // error
| ^^^^^^^^^^^^^
| test
| This location contains code that was inlined from Test_2.scala:8
| This location contains code that was inlined from Macro_1.scala:7
|---------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|·····················································································································
|This location contains code that was inlined from Test_2.scala:8
7 | notNull(i)
| ^^^^^^^^^^
·---------------------------------------------------------------------------------------------------------------------
16 changes: 16 additions & 0 deletions tests/neg-macros/i13991.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

-- Error: tests/neg-macros/i13991/Test_2.scala:6:5 ---------------------------------------------------------------------
6 | v2 // error
| ^^
| Error
|---------------------------------------------------------------------------------------------------------------------
|Inline stack trace
|·····················································································································
|This location contains code that was inlined from Test_2.scala:3
3 | inline def v2 = InlineMac.sample("foo")
| ^^^^^
|·····················································································································
|This location contains code that was inlined from Test_2.scala:3
3 | inline def v2 = InlineMac.sample("foo")
| ^^^^^^^^^^^^^^^^^^^^^^^
·---------------------------------------------------------------------------------------------------------------------
Loading