Correct offset for single-string interpolation #8629
Conversation
fix is a big word -- I'm not sure where it's pointing now is any better than where it was.
compiler/test-resources/repl/i6676
Outdated
| @@ -7,8 +7,8 @@ scala> xml" | |||
| | ';' expected, but eof found | |||
| scala> xml"" | |||
| 1 | xml"" | |||
| | ^ | |||
| | value xml is not a member of StringContext | |||
| | ^ | |||
There was a problem hiding this comment.
This is now aligned with the implementation, but it still looks wrong to me. Analogous to
scala> "".xml
1 |"".xml
|^^^^^^
|value xml is not a member of String
I would have expected
scala> "".xml
1 |xml""
|^^^^^
|value xml is not a member of StringContext
There was a problem hiding this comment.
Indeed, that would be the correct position.
Change Desugar.scala#L1668 to
Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems)
compiler/test-resources/repl/i6676
Outdated
| @@ -7,8 +7,8 @@ scala> xml" | |||
| | ';' expected, but eof found | |||
| scala> xml"" | |||
| 1 | xml"" | |||
| | ^ | |||
| | value xml is not a member of StringContext | |||
| | ^ | |||
There was a problem hiding this comment.
Indeed, that would be the correct position.
Change Desugar.scala#L1668 to
Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems)
martijnhoekstra
force-pushed
the
interpolatedPos
branch
from
a9bec78 to
506a9c4
Compare
|
@nicolasstucki that led to some semanticdb updateExpect stuff that I'm not sure looks entirely right, even though I have no idea who it should look. |
|
It looks kind of fine. @bishabosha could you have a look at the diffs in the |
| @@ -1665,7 +1665,7 @@ object desugar { | |||
| case t => t | |||
| } | |||
| // This is a deliberate departure from scalac, where StringContext is not rooted (See #4732) | |||
| Apply(Select(Apply(scalaDot(nme.StringContext), strs), id), elems) | |||
| Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems) | |||
There was a problem hiding this comment.
We could also try this variant to see if the semanticdb test keep on working without changes.
| Apply(Select(Apply(scalaDot(nme.StringContext).withSpan(tree.span), strs), id), elems) | |
| Apply(Select(Apply(scalaDot(nme.StringContext), strs), id).withSpan(tree.span), elems) |
There was a problem hiding this comment.
Giving that a try, you get 31e9464
No more truncated string segments, but I have no idea whether that was a problem in the first place or not.
There was a problem hiding this comment.
Looks better. Maybe @bishabosha can tell us which one is better. If not I would go with this one.
There was a problem hiding this comment.
I think its better now that the actual method call has a span the length of the interpolator prefix, rather than empty but i guess its not matched exactly to the prefix, but there are other desugaring anomalies like extension methods.
So this is good.
|
Thank you @martijnhoekstra |
Fixes #8627