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

type aliases no longer work? #116

Open
bennn opened this issue Jun 15, 2023 · 2 comments
Open

type aliases no longer work? #116

bennn opened this issue Jun 15, 2023 · 2 comments

Comments

@bennn
Copy link
Contributor

bennn commented Jun 15, 2023

On cinder/3.10, fannkuch_static_lib.py and nqueens_static_lib.py give errors like this:

  File "fannkuch_static_lib.py", line 29
    count[r - 1] = r
compiler.errors.TypedSyntaxError: type mismatch: int64 cannot be assigned to dynamic

The array count is defined using a type alias:

ArrayI64 = Array[int64]
....
count: ArrayI64 = ArrayI64(range(1, nb + 1))

Changing the definition to use the type directly removes the error:

count: Array[int64] = ArrayI64(range(1, nb + 1))

That's surprising! Did something break with type aliases?

cc @vivaan2006 who helped discover this

@DinoV
Copy link
Contributor

DinoV commented Jun 24, 2023

Thanks for the report... It looks like this was caused by 6ce212a. That prevents code like:

import __static__

class C:
    def __init__(self):
        self.x = 42

def f():
    global a
    a = 42
a = C()
f()

print(a.x)

From crashing, but we'll need to come up with a better solution here and retain the implicit type information while disallowing the conflicting assignment. We do report the invalid assignment if def f comes after the assignment to a so it's probably just adjusting how our passes run or adding a new pass.

@DinoV
Copy link
Contributor

DinoV commented Jun 26, 2023

We've come up with some other interesting examples:

class A: pass
class B(A): pass

def f():
    global x
    x = A()  # this assignment should be ok

def g(arg: B): pass

x: A = B()

f()

g(x)  # should be a TypedSyntaxError, with or without `f()` (or even the definition of `f`)

And we've looked at what Pyre does here as well. Both Pyre and static Python have a notion of inferred type as well as the declared type. In some annotated cases like "x = 42" it seems that there ends up being an inferred type of Literal[42] and a declared type of int.

Currently we only end up with an inferred type, and then we're turning that into dynamic. We probably need to more closely mimic what Pyre is doing for getting a declared type in these situations, and then drop the inferred type. And we need to understand more when we should treat the inferred type as declared.

We also may be able to do this in limited scenarios, e.g. where there are no references to global in the module, where we know the value won't change (because the module is known to be immutable).

And we may need to add an extra pass in to accomodate this as well.

Unfortunately this will take a little while to get to!

bennn added a commit to utahplt/static-python-perf that referenced this issue Jul 21, 2023
facebook-github-bot pushed a commit to facebookincubator/cinderx that referenced this issue Feb 7, 2025
Summary:
Treat module-level assignments where the RHS is a type as equivalent to class declarations.

Type aliases stopped working when we decided not to store inferred types for
top-level variables, due to corner cases with `global` later mutating those
variables (see facebookincubator/cinder#116 for
discussion).

However, in this specific case, it should be safe to treat the aliases as
declared rather than inferred, since their values will be used by type
annotations before we run any functions that could modify them via `global`.

Reviewed By: alexmalyshev

Differential Revision: D69254419

fbshipit-source-id: d29a268ed15a035dbc1a3f4d1a2eb32280637e45
facebook-github-bot pushed a commit to facebookincubator/cinderx that referenced this issue Feb 13, 2025
Summary:
Treat module-level assignments where the RHS is a type as equivalent to class declarations.

Type aliases stopped working when we decided not to store inferred types for
top-level variables, due to corner cases with `global` later mutating those
variables (see facebookincubator/cinder#116 for
discussion).

However, in this specific case, it should be safe to treat the aliases as
declared rather than inferred, since their values will be used by type
annotations before we run any functions that could modify them via `global`.

NOTE: Relanding of D69254419

Reviewed By: DinoV

Differential Revision: D69488212

fbshipit-source-id: 419c2d53952cbb5ebfad290b0d2c8764734d5bbd
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