[mypyc] Fix __dict__ in CPython 3.11

[Fidget-Spinner]

Fixes mypyc/mypyc#943.

Many thanks to @kumaraditya303 for pointing me in the right direction.

[JelleZijlstra]

I think this isn't right: the running version of Python doesn't necessarily equal the version used at runtime. Maybe we need to emit #ifdef'ed C code instead?

[Fidget-Spinner]

Oh right. Yeah my bad.

[ichard26]

Currently mypyc does not support cross Python version compilation, but yes conditional C code is probably better since cross version compilation is indeed a future goal. The Emitter class does have the capi_version attribute for future reference (it's currently always set to the running version of Python, could be changed down the road).

[Fidget-Spinner]

For some reason the PY_VERSION_HEX checks are not working (I suspect the appropriate headers aren't being included properly). So for now I'm using capi_version and leaving TODOs to use PY_VERSION_HEX in the future.

[Fidget-Spinner]

I have no clue how/why everything is now segfaulting.

[Fidget-Spinner]

A second pair of eyes on the C code would be much appreciated :).

[Fidget-Spinner]

🎉 All the tests are green. testProtocol is fixed in 3.11. Leaving only testProperty broken in 3.11.

[Fidget-Spinner]

This fix will only work in 3.11b5 and above. It does not work for 3.11b4. So I'm waiting for GH actions/setup-python to update to 3.11b5.

[Fidget-Spinner]

How do I make the CI use 3.11b5? @JelleZijlstra or @ichard26 do you have any idea?

[hauntsaninja]

I think it requires actions/python-versions#180 to be merged

[Fidget-Spinner]

This should now be ready. Thanks to everyone involved!

[kumaraditya303]

Not a mypyc expert but C code LGTM. I left one nit for you to consider.

[hauntsaninja]

Thank you! Looks like the run against Python 3.11b5 happened and this has fixed testProtocol and testPickling

[hauntsaninja]

Thanks, after reading python/cpython#92678 this makes sense to me. I'll leave open for a couple days in case anyone else more familiar with mypyc or CPython wants to chime in :-)

[Fidget-Spinner]

We don't need this anymore. It was fixed upstream in CPython 3.11.

[ilevkivskyi]

Hm, @Fidget-Spinner I think we may still need this PR to fix testPickling (the last failing test after #13787), see https://github.com/python/mypy/actions/runs/3165194624/jobs/5154028986

The test fails because of __dict__ handling (looks like wrong offset on Python 3.11). I started working on my own PR, but then remembered that we had this (more thorough) one. Would you like to adapt this PR to fix that last test case?

[Fidget-Spinner]

Sorry but I'll be busy the next week, so I don't think I can work on this anytime soon.

I give full consent to copy my code in this PR for use in your PR. Just attribute it with Co-Authored-By if you do use it please.

[ilevkivskyi]

@Fidget-Spinner I tried and it looks like this approach doesn't work. The problem is that _PyObject_VisitManagedDict and _PyObject_ClearManagedDict are not public in 3.11 (I am not sure what was the reason to revert the backport, is it possible to get the "better" backport from 3.12? cc @markshannon). And even if try to vendor them and include pycore_object.h (which is already sketchy), I still get errors (e.g. conflicting types for _PyObject_LookupSpecial).

Since this used to work until 3.10, it would be great if there was a reasonable way to support this in 3.11, does anyone have any ideas?

[ilevkivskyi]

Actually after looking a bit more at this, I don't think I understand what is going on here. A simple repro mypyc test case is

[case test311Repro]
from typing import Any

def dec(x: Any) -> Any:
    return x

@dec
class D: pass

class E(D):
    y: int

[file driver.py]
from native import E

e = E()
e.x = 1
e.__dict__

This raises an AttributeError on Python 3.11, while it works on Python 3.10. What we do here is "compiled class that inherits from interpreted one". D is created by simply calling type("D", ...) while E is created using a static type that has a member descriptor __dict__ pointing right after E objects struct end.

Note that assigning attributes on e still works, so it does have a dict somewhere. Maybe D gets this managed dict flag, and therefore the negative dict offset is somehow used when storing attributes on e (instead of the one right after the object)? Also it may be a bug in CPyType_FromTemplate().

@msullivan Maybe you will have time to look at this to figure out what is going on here?

[ilevkivskyi]

OK, so my guess was right, since E subclasses an interpreted class, it gets the managed dict. So the test is fixed by simply setting t->ht_type.tp_flags &= ~Py_TPFLAGS_MANAGED_DICT after E is created. I am not sure how safe it is, it seems like at least some code assumes that heap type implies managed dict.

If this hack gets us through 3.11 this is fine probably, since 3.12 already includes good public APIs to work with managed dicts.