REPL gives stack overflow during too eager syntax highlighting, e.g. when multiplying string

[bjornregnell]

Compiler version

3.3.0-RC2

Minimized code

$ scala-cli repl -S 3.3.0-RC2
Welcome to Scala 3.3.0-RC2 (17.0.4.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                                                       
scala> "works fine" * 100
val res0: String = works fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks fineworks ...
                                                                                                                                                                       
scala> "works not fine"* 1000
Exception in thread "main" java.lang.StackOverflowError
    at dotty.tools.dotc.ast.untpd$UntypedTreeAccumulator.foldMoreCases(untpd.scala:799)
    at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1641)
    at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
    at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
    at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
    at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
 

Expectation

Should work and not give stack overflow (as in e.g. 2.13)

[som-snytt]

This is the highlight of my day.

[mpilquist]

Note this isn't unique to string multiplication - it's a bug in syntax highlighting of very long strings.

[som-snytt]

it's not trying to highlight a string but a huge infix expression.

val res0: String = works fineworks fineworks

The fix is to put the string in quotes for parsing, as in https://github.com/scala/scala/pull/8885

Then, to pun further, the fix is in.

I was curious about ellipsis output:

ending in the ellipsis, so it must abort highlighting on parse error.

[WojciechMazur]

Bisect points to 1475a6f63cbbfd07a5e354bfd919c7ae9477bd1a

[prolativ]

It looks like the problem is that the limit of characters used for truncation got raised from 1000 to 50000. I think we should make the default 1000 again (3000 seem to work for me but I'm not sure if this would work universally). If users wanted to have a higher limit, they could use the setting explicitly now. CC @mpollmeier

[mpollmeier]

Sure we can change the default back to 1000, but that's only covering the underlying issue, doesn't it? As @mpilquist mentioned, it's a bug in the syntax highlighting. So if users configure a higher limit for their session, they're still going to run into this...

[som-snytt]

My point was ignored.

This is already wrong color, string would be red:

It's parsing that text to know how to colorize it, and works fineworks fineworks is an infix expression, not a string.

There are other workarounds besides putting quotes around the string, namely, not attempting to highlight the RHS or using the type to pick a color.

[bjornregnell]

Why is the interior of a string syntax highlighted in the first place? It is just a string... (sorry if I'm missing something here and asking a strange question)

[bjornregnell]

I mean: this is the algorithm that I (perhaps simplistically) assumed:

• val replInput: String = """"works fine" * 1000"""
• val code: FancyAST = intepret(replInput)
• aha code is just a string (although loooong), so no further highlighting is necessary and just print (parts of) it

(But again, I am perhaps missing something obvious... )

[prolativ]

Oh, right, the fact that printed string literals are trying to be highlighted like code seems to be the core problem here

[som-snytt]

Another good example where the color is nonsensical:

[bjornregnell]

Syntax highlighting of parts of the hash code seems all too eager. Maybe that's another bug?

[SethTisue]

the fact that printed string literals are trying to be highlighted like code seems to be the core problem here

agree. (and with Bjorn's remark also, which shows that the problem isn't limited to string literals.)

I'm going to optimistically label this with “spree”, since the issue shouldn't require tearing up much of the REPL code? I hope?

[mpollmeier]

Apart from the rendering being buggy, a major problem is that the rendering first transforms the results into a String (i.e. throwing away all type information), and then trying to colorise that String.

The type information is useful not only for less-buggy rendering and coloring, but also allows us to enrich the output with product labels etc. Here's an RFC that demonstrates one way to fix things, by using pprint: https://github.com/lampepfl/dotty/pull/17624

[mbovel]

Similar issue: https://github.com/lampepfl/dotty/issues/18596.

Not sure if there are some other bugs at hand here, but isn't it unavoidable that the highlighter crashes on too deeply nested expressions?

Should we just catch StackOverflows and report an error? Or should we not highlight strings larger than some threshold?

[bjornregnell]

Refraining from highlighting beyond some threshold is good I think. Better to be robust and not crash.

[som-snytt]

Or just display strings in quotes like everyone asks for & has received PRs for.

[mbovel]

Or just display strings in quotes like everyone asks for & has received PRs for.

I couldn't agree more. Why don't we have quotes yet; were there prior discussions about this?

However that would not solve the general problem of stackoverflows when highlighting deeply nested expression. I guess we can get the same problem without strings.

[som-snytt]

I haven't finished my second cup of coffee to answer what complex thing it knows how to render. Huge case classes? I did already do wordle etc.

scala> val x = 42 * 2
val x: Int = 84

scala> case class C(i: Int, j: Int)
// defined case class C

scala> C(42*2, 27)
val res0: C = C(84,27)

The reason to highlight as code is that it is trying to render something code-like, but the RHS is just a value that is rendered. Oh, large collections is an obvious example of potentially large rendering. But that is "long" and not "deep". The other would be parse trees as deeply nested case classes, if people do that in REPL.

[mbovel]

The other would be parse trees as deeply nested case classes, if people do that in REPL.

Indeed. I tried it and actually got a stack overflow in the parser already. We could also safe-guard it.

➜  ~/dotty git:(mb/refinements) scala-cli repl -S "3.3.2" --jvm 17
Welcome to Scala 3.3.2 (17.0.6, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                     
scala> case class Tree(left: Tree, right: Tree)
// defined case class Tree
                                                                                                     
scala> def deepTree(depth: Int): Tree = if depth == 0 then null else Tree(null, deepTree(depth - 1))
def deepTree(depth: Int): Tree
                                                                                                     
scala> deepTree(300)
Exception in thread "main" java.lang.StackOverflowError
	at dotty.tools.dotc.parsing.Scanners$Region.toList(Scanners.scala:1618)
	at dotty.tools.dotc.parsing.Scanners$Region.toList(Scanners.scala:1618)
	at dotty.tools.dotc.parsing.Scanners$Region.toList(Scanners.scala:1618)
        ...