improvement: use heuristic to figure out nameSpan if pointDelta too big
#22484
Conversation
| | case Thicket2(_) => ??? | ||
| | } | ||
| | | ||
| | /** Does `tree' look like a reference to AnyVal? Temporary test before we have |
There was a problem hiding this comment.
Could we skip the scaladoc actually, or make them a bit shorter?
There was a problem hiding this comment.
The problem is that the span is too big, so all these comments to make the method extra long are needed.
There was a problem hiding this comment.
Ach, makes sense. Thanks for explaining!
There was a problem hiding this comment.
It definitely falls under "things I would not have anticipated"!
| | // prefixed by type or val). `tparams` and `vparamss` are the type parameters that | ||
| | // go in `constr`, the constructor after desugaring. | ||
| | 1 | ||
| |}.<<showing2>>("aaa") |
There was a problem hiding this comment.
The > 4K string could be generated "x" * 4096 instead of adding text to the repo.
There was a problem hiding this comment.
@kasiaMarek Let's maybe to that and then merge?
| @@ -59,6 +59,9 @@ object Spans { | |||
| if (poff == SyntheticPointDelta) start else start + poff | |||
| } | |||
|
|
|||
| def pointMayBeIncorrect = | |||
| pointDelta == 0 && end - start >= SyntheticPointDelta | |||
There was a problem hiding this comment.
Is it dubious to make this API of span? Why this check on pointDelta if any large span may make it overflow?
There was a problem hiding this comment.
If pointDelta it too large, pointDelta will be set to 0, so only then it can be wrong.
There was a problem hiding this comment.
I withdraw my objection to adding the method to Span. As a footnote, there is no range check on point, so "incorrect" code can attempt to assign a point "outside" the span. This was an ongoing issue in Scala 2. I haven't looked again yet at the "multiple vars" position issue, but that is the sort of incorrect code I mean.
| val realName = name.stripModuleClassSuffix.lastPart | ||
| val probablyPoint = span.end - realName.length | ||
| Span(probablyPoint, span.end, probablyPoint) | ||
| else Span(point, span.end, point) |
There was a problem hiding this comment.
I don't even see where select gets its point; I see the span is set in typedSelectOnTerm for example. I haven't looked at the span computations there. But, it seems to me I can never trust the point of a huge span, and that counting back from the end is always correct in that case. Or, is there logic somewhere that overflowing point is reset to zero?
There was a problem hiding this comment.
Yes, overflowing is set to zero. The exact line is in the PR description.
There was a problem hiding this comment.
Thanks, my eyes saw it but my brain did not.
There was a problem hiding this comment.
For this particular case, the span is set in selectorOrMatch in Parsers.scala by atSpan method.
kasiaMarek
changed the title
nameSpan if pointDelta to bignameSpan if pointDelta too big
This is a hacky workaround when
pointDelta >= SyntheticPointDelta, see:scala3/compiler/src/dotty/tools/dotc/util/Spans.scala
Line 161 in 6d9b0f1
Fix for: scalameta/metals#3053