Class values as CheckedDict type-arguments are not handled properly

[LuKuangChen]

9186730 2021-12-01

This issue might be related to https://github.com/facebookincubator/cinder/issues/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]

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]

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]

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.

[DinoV]

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.

[DinoV]

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.