Skip to content

[mypyc] fix: builder crashes on 3-arg range(x, y, z) if step isnt foldable #20098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[mypyc] fix: builder crashes on 3-arg range(x, y, z) if step isnt foldable #20098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cf1470b
[mypyc] fix: builder crashes on 3-arg range if step isnt foldable
BobTheBuidler Oct 21, 2025
2b0407b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 21, 2025
9efe377
Update for_helpers.py
BobTheBuidler Oct 21, 2025
1dbaa15
Update for_helpers.py
BobTheBuidler Oct 21, 2025
a266982
Update for_helpers.py
BobTheBuidler Oct 21, 2025
b5d519c
Update commandline.test
BobTheBuidler Oct 21, 2025
4fda932
Update commandline.test
BobTheBuidler Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 25 additions & 33 deletions mypyc/irbuild/for_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from __future__ import annotations

from typing import Callable, ClassVar
from typing import Callable, ClassVar, cast

from mypy.nodes import (
ARG_POS,
Expand Down Expand Up @@ -454,39 +454,31 @@ def make_for_loop_generator(
return for_dict

if isinstance(expr, CallExpr) and isinstance(expr.callee, RefExpr):
if (
is_range_ref(expr.callee)
and (
len(expr.args) <= 2
or (len(expr.args) == 3 and builder.extract_int(expr.args[2]) is not None)
Copy link
Collaborator

@A5rocks A5rocks Oct 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, why did this fail with an assertion error? (I'm not used to mypyc...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, I'm realizing while re-reading this that it doesn't

I discovered this assert when working on #20100 and made this PR as an intermediate step which could be merged in before the full feature.

In hindsight I suppose this is no longer necessary, but I'll leave it open so maintainers can decide if we want to merge it before #20100

)
and set(expr.arg_kinds) == {ARG_POS}
):
# Special case "for x in range(...)".
# We support the 3 arg form but only for int literals, since it doesn't
# seem worth the hassle of supporting dynamically determining which
# direction of comparison to do.
if len(expr.args) == 1:
start_reg: Value = Integer(0)
end_reg = builder.accept(expr.args[0])
else:
start_reg = builder.accept(expr.args[0])
end_reg = builder.accept(expr.args[1])
if len(expr.args) == 3:
step = builder.extract_int(expr.args[2])
assert step is not None
if step == 0:
Copy link
Contributor Author

@BobTheBuidler BobTheBuidler Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we no longer need to check this because we validate that it isn't None or 0 in our new if-check. 0 now falls back to the standard implementation where it fails normally

builder.error("range() step can't be zero", expr.args[2].line)
else:
step = 1
num_args = len(expr.args)

for_range = ForRange(builder, index, body_block, loop_exit, line, nested)
for_range.init(start_reg, end_reg, step)
return for_range
if is_range_ref(expr.callee) and set(expr.arg_kinds) == {ARG_POS}:
# Special case "for x in range(...)".
# NOTE We support the 3 arg form but only when `step` is constant-
# foldable, since it doesn't seem worth the hassle of supporting
# dynamically determining which direction of comparison to do.
# If we cannot constant fold `step`, we just fallback to stdlib range.
if num_args <= 2 or (num_args == 3 and builder.extract_int(expr.args[2])):
if num_args == 1:
start_reg: Value = Integer(0)
end_reg = builder.accept(expr.args[0])
step = 1
else:
start_reg = builder.accept(expr.args[0])
end_reg = builder.accept(expr.args[1])
step = 1 if num_args == 2 else cast(int, builder.extract_int(expr.args[2]))

for_range = ForRange(builder, index, body_block, loop_exit, line, nested)
for_range.init(start_reg, end_reg, step)
return for_range

elif (
expr.callee.fullname == "builtins.enumerate"
and len(expr.args) == 1
and num_args == 1
and expr.arg_kinds == [ARG_POS]
and isinstance(index, TupleExpr)
and len(index.items) == 2
Expand All @@ -500,10 +492,10 @@ def make_for_loop_generator(

elif (
expr.callee.fullname == "builtins.zip"
and len(expr.args) >= 2
and num_args >= 2
and set(expr.arg_kinds) == {ARG_POS}
and isinstance(index, TupleExpr)
and len(index.items) == len(expr.args)
and len(index.items) == num_args
):
# Special case "for x, y in zip(a, b)".
for_zip = ForZip(builder, index, body_block, loop_exit, line, nested)
Expand All @@ -512,7 +504,7 @@ def make_for_loop_generator(

if (
expr.callee.fullname == "builtins.reversed"
and len(expr.args) == 1
and num_args == 1
and expr.arg_kinds == [ARG_POS]
and is_sequence_rprimitive(builder.node_type(expr.args[0]))
):
Expand Down
2 changes: 1 addition & 1 deletion mypyc/test-data/commandline.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ from typing import Final, List, Any, AsyncIterable
from mypy_extensions import trait, mypyc_attr

def busted(b: bool) -> None:
for i in range(1, 10, 0): # E: range() step can't be zero
for i in range(1, 10):
try:
if i == 5:
break # E: break inside try/finally block is unimplemented
Expand Down