Scala: allow ellipsis in for loop headers #5661
Conversation
PA-1579 Scala for loops parse error
I expected that would parse as patterns but they don't as of 0.100: https://semgrep.dev/s/NoNx |
|
great PR comment! |
| and multi_for_each = | ||
| | FE of for_each | ||
| | FECond of for_each * tok * expr | ||
| | FEllipsis of tok |
There was a problem hiding this comment.
I usually add a semgrep-ext: comment before those constructs that are Semgrep specific.
|
|
||
| and multi_for_each = | ||
| | FE of for_each | ||
| | FECond of for_each * tok * expr |
There was a problem hiding this comment.
maybe in comment you can put an example, like x <- xs if x > 2
| | G.FECond (a1, at, a2), B.FECond (b1, bt, b2) -> | ||
| m_for_each a1 b1 >>= fun () -> | ||
| m_tok at bt >>= fun () -> m_expr a2 b2 | ||
| | G.FEllipsis _, _ -> return () |
There was a problem hiding this comment.
I don't think this can can happen given the definition of m_list_with_dots, but I guess it does not hurt.
| G.If (g_tok, G.Cond g_e, stmt, None) |> G.s) | ||
| guards stmt) | ||
| v2 v3 | ||
| G.For (v1, G.MultiForEach v2, v3) |> G.s |
There was a problem hiding this comment.
I usually try to avoid adding a construct in AST_generic when it's useful only for one language, but
maybe it's ok for this one indeed.
| @@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| ### Added | |||
|
|
|||
| - Scala: ellipsis are now allowed in for loop headers (#5650) | |||
There was a problem hiding this comment.
During the "changelog meeting" this week, Bruno said he is now the one who is looking at the changelog to help producing the release notes, and that he thinks we don't generally write good changelog entries (that allow him to do that job easily). I think he will have problems understanding the purpose of this one.
There was a problem hiding this comment.
You can ask him directly, but my understanding is that he would like to know that this change is to allow Scala users to use ellipsis to write patterns matching a sequence of nested for loops. He actually emphasized a lot the what, why, and how but I'm not sure how that would be instantiated in this particular case.
There was a problem hiding this comment.
I think an example would be nice "(e.g., you can write a pattern like for(...; $X <- bar(); ...) { ... }".
Otherwise I feel the associated issue (#5650) mentioned in the changelog can help answer the what/why/how.
Fixes #5650
This PR adds support for Scala patterns like
for (...; $X <- $Y if $COND; ...) { ... }. These are supported by adding a newMultiForEachfor loop header to the generic AST. In the AST to IL translation, this construct is unrolled into nested for loops. Extending the generic AST to treat Scala's for loop headers as a list was more straightforward than attempting to do some funky matching involving big nested for loops.This PR also deletes the
GIfconstructor for enumerators (for loop headers) from the Scala AST. As far as I can tell this was never actually produced by the parser, as guards are always parsed as directly part of a generator (x <- y). The Scala docs agree with me: https://scala-lang.org/files/archive/spec/2.13/06-expressions.html#for-comprehensions-and-for-loops. There is no mention of these guards that are not part of a generator.test plan: make test
PR checklist: