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

Confusing int64 error at cyclic class boundary #146

Open
bennn opened this issue Oct 22, 2024 · 2 comments
Open

Confusing int64 error at cyclic class boundary #146

bennn opened this issue Oct 22, 2024 · 2 comments

Comments

@bennn
Copy link
Contributor

bennn commented Oct 22, 2024

I have a method that expects an int64, but when I call it from another, special location the error says expected an int.

The other location is across a cyclic dependency: file test1 imports file test2, but the class inside test2 expects an object defined in test1 as an init param:

## test2.py , home of class A

from __future__ import annotations
from __static__ import int64

class A:
    def __init__(self, b: Board):
        self.b: Board = b

    def add(self):
        self.b.add(int64(0))
## test1.py , home of class Board

from __static__ import int64
from test2 import A

class Board:
    def add(self, pos: int64) -> None:
        return

def main():
    bb = Board()
    A(bb).add()
    return

main()
$ [static-python] test1.py
  ....
  File "/vol/static-python-perf/Benchmark/go/advanced/test2.py", line 9, in add
    self.b.add(int64(0))
TypeError: add expected 'int' for argument pos, got 'int64'

The correct error is that call arguments cannot be primitive --- right? That's what I see without the cycle:

## test3

import static

class Board:
def add(self, pos: int64) -> None:
return


## test4

from static import int64
from test3 import Board

class A:
def init(self, b: Board):
self.b: Board = b

def add(self):
    self.b.add(int64(0))

def main():
bb = Board()
A(bb).add()
return

main()


$ [static-python] test4.py
 ....
    raise exception
  File "test4.py", line 9
    self.b.add(int64(0))
              ^
cinderx.compiler.errors.TypedSyntaxError: Call argument cannot be a primitive
@DinoV
Copy link
Contributor

DinoV commented Oct 22, 2024

Thanks for the bug report!

I think there's a couple of things here. First, in test2.py, you'll need to let us know what the types are somehow, so you do need the import. The way to deal with cyclical imports in this case is:

from typing import TYPE_CHECKING
if TYPE_CHECKING:
    from test1 import Board

I expect that'll fix it up. But the 2nd thing is that you shouldn't need the int64(0), you can just use 0 instead and the int64 part of it will be implied.

That being said I do think we we have an issue here. We should probably make a couple of changes. First an explicit call to int64(...) in a place where we're passing things dynamically should pass an int instead of creating the fake int64 type.

The 2nd is that we should probably allow the int64 type to be passed to this function. Currently we're saying this is an exact type but that's not really true as we should allow sub-types of ints as well.

Just a heads up but we've been working on making Cinder a standalone extension. So right now our active development for the static Python portion of things is happening over at https://github.com/facebookincubator/cinderx. But we still require a forked runtime which is not the default branch here, it's https://github.com/facebookincubator/cinder/tree/meta/3.10.cinder. Unfortunately we don't quite yet have a public build story going so it'll be hard to pick up a bug fix but we'll work on getting this fixed in the current branch :(

@bennn
Copy link
Contributor Author

bennn commented Oct 22, 2024

Ok, thanks for the answers and big thanks for the heads up! We'll watch the cinderx repo & wait for a bit before running serious experiments.

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

No branches or pull requests

2 participants