Skip to content

fix: replace a comprehension variable only where it is a whole identifier - #66

Merged
ylluminate merged 1 commit into
vlang:masterfrom
ylluminate:fix-comprehension-identifier-replace
Nov 13, 2026
Merged

ylluminate merged 1 commit into
vlang:masterfrom
ylluminate:fix-comprehension-identifier-replace

Conversation

@ylluminate

Copy link
Copy Markdown
Contributor

A list, set or generator comprehension is lowered to .filter(...)/.map(...) (unless a filter holds a walrus, which takes another path), and its loop variable is turned into V's it by replacing the variable's name in the emitted text. The replacement took every occurrence of those letters. With r as the variable, [max([r.turns, r.pred]) for r in data] came out as data.map(aititays.max([it.tuitns, it.pited]) oit { panic('!') }), [r + r_tail for r in rows] as rows.map(it + it_tail), and [r for r in rows if r > len("r")] as rows.filter(it > 'it'.len).map(it). The first two don't compile, and the third compiles and prints [3] where Python prints [2, 3].

The two replacements now go through replace_comp_identifier(), which replaces the name only where it is a whole identifier and leaves the text of string literals alone. Inside a ${...} interpolation the name is replaced in the expression ([f"r={r}" for r in rows] is rows.map('r=${it}')), but the type letter of a static format such as :d or :05.2f is left alone (with d as the variable, '${7:d}' keeps its d). The three above become data.map(arrays.max([it.turns, it.pred]) or { panic('!') }), rows.map(it + r_tail) and rows.filter(it > 'r'.len).map(it).

It is still a replacement after the element has been emitted, on purpose. py2v keys several tables on a name as it is emitted (a function's defaults, its mut parameters, Path variables), so emitting it for the variable while the element is visited changes what they see: [f() for f in fs] loses f's default argument, for one.

What it leaves as it was:

  • A target that isn't emitted as a single identifier (a tuple or starred target) is replaced as before, by its whole emitted text.
  • A field named like the variable is still rewritten ([x.r + r for r in rows] is rows.map(x.it + it)). py2v lowers getattr(x, r) to x.r, so a name after a dot can be the variable itself, and the text doesn't say which.
  • A second for clause is still dropped, and a nested comprehension that uses the outer variable still reads the inner it ([[r + s for s in [1, 2]] for r in rows] is rows.map([1, 2].map(it + it))).
  • The match-capture guard does the same kind of replacement (with a subject s, case r if r.startswith("a") comes out as s.stasts_with('a')), which is a separate fix.

This needed #57 first. Without it, ["{0[r]}".format({"r": r}) for r in ["it"]] would go from right to wrong here: it was only right because the replacement also rewrote the quoted key.

Test plan

  • v run tests/run_tests.vsh: 179 passed, 0 failed, 0 skipped (V 0.5.2 ec510e2, macOS), with the new case comprehension_identifier_replace. No existing expected file changes.
  • v test py2v_test.v passes with the new tests of replace_comp_identifier().
  • I ran generated programs through CPython, master and this branch, with V's retry on its fallback compiler turned off (V_MACOS_V3_NO_FALLBACK=1) and v -W run. Of 686 comprehension and lambda programs, 84 that failed to compile or printed the wrong thing before now print what CPython prints (43 and 41), the 202 that matched still match, and 400 match on neither side. Eight of those 400 didn't compile before and now compile and print the wrong thing, for reasons this leaves alone: an enclosing variable named it (three), the dropped second for (two), the nested it, a set emitted as an array, and a walrus result typed Any. Five other sets (445, 188, 316, 280 and 489 programs) end the same way with and without this change.

…fier

A list, set or generator comprehension is lowered to
`.filter(...)`/`.map(...)` (unless a filter holds a walrus), and its
loop variable is turned into V's `it` by replacing the variable's name
in the emitted text. The replacement took every occurrence of those
letters, so with `r` as the variable
`[max([r.turns, r.pred]) for r in rows]` came out as
`rows.map(aititays.max([it.tuitns, it.pited]) oit { panic('!') })`,
`[r + r_tail for r in rows]` as `rows.map(it + it_tail)`, and
`[r for r in rows if r > len("r")]` as
`rows.filter(it > 'it'.len).map(it)`.

replace_comp_identifier() replaces the variable only where it is a
whole identifier, and leaves the text of string literals alone. Inside
a `${...}` interpolation the name is replaced in the expression, but
the type letter of a static format such as `:05d` is left alone. A
target that is not emitted as a single identifier (a tuple or starred
target) is replaced as before.
@ylluminate
ylluminate merged commit 089ebfa into vlang:master Nov 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant