[PEP 558 - WIP] bpo-30744: Trace hooks no longer reset closure state by ncoghlan · Pull Request #3640 · python/cpython (original) (raw)

I have a concern about raising RuntimeError from PyFrame_LocalsToFast.

Was it possible for PyFrame_LocalsToFast before this change to ever raise RuntimeError for any other reason?

If not, then it would be nice to have that explicitly documented (at least in the PEP, maybe elsewhere if appropriate and not too noisy to include it).

But RuntimeError is raised for internal problems, like if memory allocations fail, right? So it seems like the kind of error type that's liable to fly out of any internal function, with no real guarantees that it can't?

So I think it might be a bit of a challenge for third parties to write code that

  1. work on Python versions before this change,
  2. will work on later Python version after this change, and
  3. won't ever hide any other errors on versions before this change, and
  4. does all that reasonably cleanly.

Because it seems like code released today:

  1. can't start catching just any RuntimeError from PyFrame_LocalsToFast,
  2. doesn't have guarantees about exactly what subclass or error message of RuntimeError to expect based on this PEP, and
  3. can't just check sys.version_info because it seems like we don't have certainty about when this PEP will make it in.

For what it's worth, this isn't insurmountable - in my little toy library that mutates functions' variables through f_locals, I settled on trying to modify a variable in the frame of a test function call, checking if that worked, and only if it didn't, I try to import the locals-to-fast stuff. I think this is probably the best in terms of portability across all versions of all implementations of Python where mutating a frame's locals is possible, and elegantly avoids ever causing the RuntimeError that this PEP adds on any implementation+version of Python that implements this PEP. But it takes all this code, might be fragile in other ways or have other downsides I haven't realized yet, and wasn't trivial to think of.

TL;DR: because change takes PyFrame_LocalsToFast from necessary for certain goals to unusable, the added error should be programmatically easy to distinguish for the code that was using it while it was necessary.