Fix SI-8202 and improve support for splicing patterns into vals #3455
Conversation
ghost
assigned
| qname | ||
| else if (sym.name.toTermName == nme.ERROR) | ||
| s"<$qname: error>" |
There was a problem hiding this comment.
Used to NPE on me before the change.
There was a problem hiding this comment.
That method is not available there.
There was a problem hiding this comment.
What do you mean? It's reflect/internal/Printers.
There was a problem hiding this comment.
Must have been a glitch, I've used it there and had compilation error. Not anymore.
|
Why not parse |
|
@xeno-by I'd love to see that happen. But it looks like there are some concerns about change of behavior in existing code. We need to discuss it one of next week's meetings. |
|
@retronym doesn't like the fact that code like: val (x: Int) = 1Will change meaning from 2.10 to 2.11 (compile-time check will become run-time check.) It will be 100% source compatible though and I doubt that anyone uses such a form anyway considering the fact that it's exactly the same as the one without parens atm. |
|
ping @xeno-by |
|
What are the performance implications of this pull request? |
|
After discussion today the conclusion was that the behavior of |
|
Thinking about it some more, it seems we can likely just fix this in 2.12 outright, ideally with an opt-in warning that lets users detect where their program is affected. (As part of the style/lint checker) |
|
(Where "this" is consistent interpretation of tuple patterns in val and for) |
|
@xeno-by what kind of performance are you talking about? Runtime isn't affected in any way. Compile-time of the quasiquote will perform implodePatDefs transform on every parsed tree. |
|
@densh I see. Thanks |
|
@adriaanm Sounds good. Looking forward too it. |
Previously construction logic used to be in Parsers and deconstruction in Placeholders making it easy to forget one if you change the other.
Also use sym.isErrorneous instead of manual name check.
This commits adds construction-only support for splicing patterns into vals (a.k.a. PatDef). Due to non-locality of the desugaring it would have been quite expensive to support deconstruction as the only way to do it with current trees is to perform implodePatDefs transformation on every single tree.
|
@xeno-by Just rebased and added some extra tests to support |
|
ping @xeno-by, please merge early to prevent rebasing issues with reflection pr |
|
LGTM |
Fix SI-8202 and improve support for splicing patterns into vals
Fixes:
review @xeno-by @retronym