Repository navigation
feat: new mypyc primitives for weakref.proxy - #19217
JukkaL merged 32 commits into
Conversation
Sorry, something went wrong.
This comment was marked as resolved.
|
Tests are fixed, this PR is now ready for review. |
Sorry, something went wrong.
for more information, see https://pre-commit.ci
|
Thanks for the feedbacks, I've addressed all of them and this PR is again ready for review. |
Sorry, something went wrong.
|
Hmm.. what happened here? the testutil import broke the tests? |
Sorry, something went wrong.
|
@JukkaL any idea why replacing pytest.raises with assertRaises is giving us all of these type-related errors? its another typeshed thing? |
Sorry, something went wrong.
| class list: ... | ||
| class BaseException: ... | ||
| class Exception(BaseException): ... | ||
| class ReferenceError(Exception): ... |
There was a problem hiding this comment.
This fixture is probably causing the problems -- some of these definitions are too simplified (or missing). If ReferenceError is the only new thing you need here, what about adding it to the generic ir.py?
Sorry, something went wrong.
There was a problem hiding this comment.
Ah, I saw a comment somewhere indicating not to add things to that file as it would slow down the whole suite.
No worries, I've updated the PR accordingly and the tests pass now.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, we prefer not to add too many things to the file, but sometimes it's just too much effort to avoid doing this.
Sorry, something went wrong.
|
I've added -> None to the test defs, should be good to go |
Sorry, something went wrong.
|
Some tests are failing. Maybe the test stubs need more realistic types? |
Sorry, something went wrong.
JukkaL
left a comment
There was a problem hiding this comment.
Looks good now!
Sorry, something went wrong.
This PR adds 2 new weakref primitives for weakref.proxy (1 and 2 arg) (cherry picked from commit 8f2371a)
This PR adds 2 new weakref primitives for weakref.proxy (1 and 2 arg)
The C code generates correctly, but I'm not entirely sure why this test is failing. The weakly-proxied object is being destroyed too early, while there should still be a strong reference to it. It also fails if we use the builtin weakref.proxy, so I believe this might be exposing a reference counting bug unrelated to this PR.