feat: Build for loops and comprehensions #68
Conversation
| illegals = find_nodes(is_illegal_in_list_comp, node) | ||
| if illegals: | ||
| raise GuppyError( | ||
| "Expression is not supported inside a list comprehension", illegals[0] |
There was a problem hiding this comment.
Should we maybe include all of them and not just the zeroth?
There was a problem hiding this comment.
It would be nice, but at the moment we can only report a single location that is highlighted in the error message (see for example here)
guppy/cfg/builder.py
Outdated
| """ | ||
| it = make_var(next(tmp_vars), g.iter) | ||
| b = make_var(next(tmp_vars), g.iter) | ||
| [gen.iter_assign, gen.hasnext_assign, gen.next_assign] = cast( |
There was a problem hiding this comment.
I find this example a lot less convincing a use of template_replace than the last one. I imagine something like
gen.iter_assign = make_assignment(it, MakeIter(value=g.iter, origin_node=node))
gen.hasnext_assign = make_assignment( (b, it), with_loc(it, IterHasNext(value=it)) )
gen.next_assign = make_assignment( (x, it), with_loc(it, IterNext(value-=it)) )
which seems a whole lot shorter and with a whole lot easier to see what ends up where without having to thread the value through kwarg-variable into the template. Now I'm sure that the practice may not be as easy as my hypothetical make_assignment (to a tuple of variables!!), but....how much worse? I'm not convinced this template_replace really pulls its weight?
There was a problem hiding this comment.
Switched to a make_assign helper function 👍
guppy/ast_util.py
Outdated
| replacements: Mapping[str, ast.AST | Sequence[ast.AST]] | ||
| default_loc: ast.AST | ||
|
|
||
| def __init__( |
There was a problem hiding this comment.
Note I think you could just make this an @dataclass and avoid the constructor. (@dataclass(eq=False, frozen=True) or similar if you want to)
ast_util.pyfile and provide a newtemplate_replacemethod to easily create AST nodes.