Skip to content
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.

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

[mypyc] Add 'range' primitive type #10307

Merged
merged 8 commits into from
Jun 8, 2021
Merged

Conversation

97littleleaf11
Copy link
Collaborator

@97littleleaf11 97littleleaf11 commented Apr 11, 2021

Description

Closes mypyc/mypyc#770

Test Plan

def range_object() -> None:
    r = range(4, 12, 2)
    sum = 0
    for i in r:
        sum += I

This should generate a range object and a iterator when looping on it.

def range_in_loop() -> None:
    sum = 0
    for i in range(4, 12, 2):
        sum += I

This should maintain original specialized IR result.

@97littleleaf11 97littleleaf11 marked this pull request as ready for review April 26, 2021 20:36
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good, just a few comments about tests.

for i in r:
sum += i

def range_in_loop() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

There probably is an existing test case for range in a for loop, so this can be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I cannot find the irbuild case and I'll keep this.

@@ -452,3 +452,19 @@ def bar(x: Optional[str]) -> None:
[file driver.py]
from native import bar
bar(None)

[case testRangeObject]
r1 = range(4, 12, 2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you change this to use a test_ function without a custom driver, as that's the "modern" way to write tests.

Test also that coercion to range works, for example something like this:

def f(r: range) -> None:
    pass

r: Any = range(...)
f(r)  # OK

r = 'x'
f(r)  # Should be a TypeError

ff: Any = f
ff(r)  # Again should be a TypeError; this uses the wrapper function since the callee has type Any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got this error after adding function def f(r: range) -> None:

run-loops.test:460: error: Function "builtins.range" is not valid as a type
run-loops.test:460: note: Perhaps you need "Callable[...]" or a callback protocol?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to change the definition of range into a class in mypyc/test-data/fixtures/ir.py.

i = r3
goto L1
L4:
return 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Two cases seem to work correctly.

@97littleleaf11 97littleleaf11 requested a review from JukkaL May 6, 2021 09:30
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

LGTM

@JukkaL JukkaL merged commit 0a6a48c into python:master Jun 8, 2021
@97littleleaf11 97littleleaf11 deleted the add-range branch June 8, 2021 11:11
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.

Add primitive type for range
2 participants