Allow pickling Cancelled

[A5rocks]

Fixes #3248

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00000%. Comparing base (737d96a) to head (9dc7386).
Report is 107 commits behind head on main.

Additional details and impacted files

@@               Coverage Diff                @@
##                 main        #3250    +/-   ##
================================================
  Coverage   100.00000%   100.00000%            
================================================
  Files             124          124            
  Lines           18848        19340   +492     
  Branches         1277         1318    +41     
================================================
+ Hits            18848        19340   +492     

Files with missing lines	Coverage Δ
src/trio/_core/_exceptions.py	100.00000% <100.00000%> (ø)
src/trio/_core/_tests/test_run.py	100.00000% <100.00000%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jakkdl]

we could instead add __reduce__ to the NoPublicConstructor metaclass to enable pickling for all objects that make use of it

[A5rocks]

we could instead add __reduce__ to the NoPublicConstructor metaclass to enable pickling for all objects that make use of it

I was thinking that too (we'd need to add it in NoPublicConstructor's __init__ or make a class we inherit from that uses NoPublicConstructor, but totally possible) but I'm not sure we want pickling for everything. I guess I can't think of any reasons against though.

[A5rocks]

Alright generally applying __reduce__ is simple and TBH it's probably fine. People could work around not having a public constructor anyways.

The main point is the number of type: ignores it uses but I assume that could be minimized if I actually cared to.

[CoolCat467]

In this case, wouldn't self be an instance of NoPublicConstructor? If so, that would probably alleviate the attr-defined error on line 335

[A5rocks]

No, Cancelled itself is an instance of NoPublicConstructor, (well not exactly cause we hand that off to type in __new__ but that's the idea) not instances of Cancelled. Metaclasses are weird!

[jakkdl]

just a couple small things, otherwise looks as good as it could given the messiness of metaclasses :)

[jakkdl]

okay I looked a bit more thorough now and found some small things :)

[A5rocks]

Turns out I underestimated pickling. It's more complicated than I expected and I don't think there's a way to generalize support to any class. I was operating in a world where Exception.__reduce__ was the default but object.__reduce__ is really weird (try it yourself!). In fact, I would need to assume what __set_state__ is doing and have special casing for if it's bypassed using a dict, and compare the first element to copyreg._reconstructor to know whether to replace the second element, and whatever. It's not worth the complexity, so I'm going to undo generalizing this. Nobody should be pickleing these anyways!

[jakkdl]

sorry for the detour ^^; Back to nice and simple