facebookincubator / cinder

Cinder is Meta's internal performance-oriented production version of CPython.
https://trycinder.com
Other
3.42k stars 122 forks source link

take infinite loops into account in type narrowing #64

Closed carljm closed 2 years ago

carljm commented 2 years ago

The fix for #62 will probably lead us to reject this program:

from typing import Optional

def f(x: Optional[str]) -> str:
    while True:
        if x is None:
            continue
        return x

print(f(None))

In order to fix this, we need to understand that the while loop does not terminate (unless there's a break) and narrow the types in the code following the loop accordingly.

LuKuangChen commented 2 years ago

Here's another program that might be affected by a fix

(The test is from Lib/test/test_compiler/test_static/tests.py)

    def test_assign_while_returns_but_assigns_first(self):
        codestr = """
            from typing import Optional
            def f(x: Optional[int]) -> int:
                y: Optional[int] = 1
                while x is None:
                    y = None
                    return 1
                return y
        """
        self.compile(codestr, modname="foo")
carljm commented 2 years ago

Thanks @LuKC1024 ! I think this program will be fine; it does not involve implicit-None-return anywhere so it should not be affected by the fix for #62, and it does not involve a definitely-infinite loop, so it should not be impacted by whatever is done for this issue.

Are you seeing an issue related to our handling of this program in your model?

LuKuangChen commented 2 years ago

Hi Carl. If it's not affected, that's great. The model has trouble with this program because the type-checker needs to know y = None won’t be effective after the while-loop exits (because the function has returned before that).

carljm commented 2 years ago

Right. The SP type checker does detect if a loop unconditionally terminates, and in that case ignores its local-type effects after the loop.

carljm commented 2 years ago

Fixed by 49e44ef93d03259e3a0c45b0a73894ff81a383c0