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

Class values as CheckedDict type-arguments are not handled properly #59

Closed
LuKuangChen opened this issue Dec 2, 2021 · 5 comments
Closed
Assignees
Labels
sp-correctness static python correctness staticpython static python issues

Comments

@LuKuangChen
Copy link

LuKuangChen commented Dec 2, 2021

9186730
2021-12-01

This issue might be related to #33

What program did you run?

from __static__ import CheckedDict
def make_C():
    class C: pass
    return C
C = make_C()
d = CheckedDict[str, C]({})

What happened?

The program raises a runtime error.

TypeError: chkdict[str, object].__new__(chkdict[str, C]): chkdict[str, C] is not a subtype of chkdict[str, object]

What should have happened?

We expected two possible outcomes:

  • a compile-time error complaining that CheckedDict[str, C] is invalid because C is not a valid type
  • a clearer run-time error
  • the program terminates without any error.
@DinoV
Copy link
Contributor

DinoV commented Dec 3, 2021

The problem here is that we think we want to create a CheckedDict[str, dynamic] because we don't know what C resolves to. But we then call CheckedDict.__new__(CheckedDict[str, C]) to create the instance of it.

@bennn
Copy link
Contributor

bennn commented Dec 3, 2021

Is chkdict[str, object].__new__ the same as CheckedDict.__new__ (the right constructor)? We were wondering how object got involved, as opposed to C or dynamic.

@carljm
Copy link
Contributor

carljm commented Dec 3, 2021

There is no dynamic at runtime, dynamic is purely a compiler construct. If a dynamic type ends up in the bytecode, it gets translated to object.

I think probably what we want here is the static type error on trying to make a CheckedDict with dynamic type as key or value type.

@carljm carljm added sp-correctness static python correctness staticpython static python issues labels Dec 3, 2021
@carljm carljm self-assigned this Dec 3, 2021
@DinoV
Copy link
Contributor

DinoV commented Dec 4, 2021

The difficulty with disallowing a dynamic type is we do want to allow it sometimes. We have things like CheckedDict[str, Set[...]]. In this case other type checkers happily see the Set[...] type and CheckedDict as an alias for Dict and enforce the appropriate types. But we just see Set[...] as dynamic, and turn it into a CheckedDict[str, object].

So I think disallowing it ends up being worse than making it work. I have a fix which would make this work for CheckedDict by getting rid of __new__ so that our allocation will go through TP_ALLOC which takes a type descriptor. Unfortunately if we ever want a generic type which is immutable (which would presumably take arguments) then we'll run into this again.

There's probably a couple of possible solutions for that. One might be to add a way to invoke __new__ that takes the type descriptor and passes the resolved type in as the 1st argument (i.e. a TP_NEW opcode). What might be better would be adding an opcode which turns a type descriptor into the underlying type. Then we could emit the __new__ call with the compiler known type instead of emitting the various loads and binary subscrs required to produce the type. That might not require any specialization of the __new__ handling - we could just use that to emit any generic type references.

@carljm carljm assigned DinoV and unassigned carljm Dec 5, 2021
@DinoV
Copy link
Contributor

DinoV commented Dec 9, 2021

This is now fixed, we'll make a CheckedDict which respects what the compiler things the type is instead of the runtime provided type. Per the discussion above there'll be more work in the future though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sp-correctness static python correctness staticpython static python issues
Projects
None yet
Development

No branches or pull requests

4 participants