Skip to content

Commit

Permalink
[mypyc] Fix evaluation of iterable in list comprehension twice
Browse files Browse the repository at this point in the history
This could result in a crash if the second evaluation results
in a shorter list, such as in this example (besides being incorrect
overall):

```
a = [s for s in f.readlines()]
```

`f.readlines()` was called twice, resulting in an empty list on
the second call. This caused the list object to have a NULL item,
which is invalid.

This should fix `mypy --install-types` in compiled mode.
  • Loading branch information
JukkaL committed Jun 8, 2021
1 parent b68993c commit 32d6b58
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
13 changes: 7 additions & 6 deletions mypyc/irbuild/for_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ def for_loop_helper(builder: IRBuilder, index: Lvalue, expr: Expression,
builder.activate_block(exit_block)


def for_loop_helper_with_index(builder: IRBuilder, index: Lvalue, expr: Expression,
def for_loop_helper_with_index(builder: IRBuilder,
index: Lvalue,
expr: Expression,
expr_reg: Value,
body_insts: Callable[[Value], None], line: int) -> None:
"""Generate IR for a sequence iteration.
Expand All @@ -101,7 +104,6 @@ def for_loop_helper_with_index(builder: IRBuilder, index: Lvalue, expr: Expressi
body_insts: a function that generates the body of the loop.
It needs a index as parameter.
"""
expr_reg = builder.accept(expr)
assert is_sequence_rprimitive(expr_reg.type)
target_type = builder.get_sequence_type(expr)

Expand Down Expand Up @@ -163,16 +165,15 @@ def sequence_from_generator_preallocate_helper(
if len(gen.sequences) == 1 and len(gen.indices) == 1 and len(gen.condlists[0]) == 0:
rtype = builder.node_type(gen.sequences[0])
if is_list_rprimitive(rtype) or is_tuple_rprimitive(rtype):

length = builder.builder.builtin_len(builder.accept(gen.sequences[0]),
gen.line, use_pyssize_t=True)
sequence = builder.accept(gen.sequences[0])
length = builder.builder.builtin_len(sequence, gen.line, use_pyssize_t=True)
target_op = empty_op_llbuilder(length, gen.line)

def set_item(item_index: Value) -> None:
e = builder.accept(gen.left_expr)
builder.call_c(set_item_op, [target_op, item_index, e], gen.line)

for_loop_helper_with_index(builder, gen.indices[0], gen.sequences[0],
for_loop_helper_with_index(builder, gen.indices[0], gen.sequences[0], sequence,
set_item, gen.line)

return target_op
Expand Down
17 changes: 16 additions & 1 deletion mypyc/test-data/run-misc.test
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,8 @@ print(native.x)
77

[case testComprehensions]
from typing import List

# A list comprehension
l = [str(x) + " " + str(y) + " " + str(x*y) for x in range(10)
if x != 6 if x != 5 for y in range(x) if y*x != 8]
Expand All @@ -498,6 +500,17 @@ def pred(x: int) -> bool:
# eventually and will raise an exception.
l2 = [x for x in range(10) if x <= 6 if pred(x)]

src = ['x']

def f() -> List[str]:
global src
res = src
src = []
return res

l3 = [s for s in f()]
l4 = [s for s in f()]

# A dictionary comprehension
d = {k: k*k for k in range(10) if k != 5 if k != 6}

Expand All @@ -506,10 +519,12 @@ s = {str(x) + " " + str(y) + " " + str(x*y) for x in range(10)
if x != 6 if x != 5 for y in range(x) if y*x != 8}

[file driver.py]
from native import l, l2, d, s
from native import l, l2, l3, l4, d, s
for a in l:
print(a)
print(tuple(l2))
assert l3 == ['x']
assert l4 == []
for k in sorted(d):
print(k, d[k])
for a in sorted(s):
Expand Down

0 comments on commit 32d6b58

Please sign in to comment.