bpo-35381 Remove all static state from posixmodule by eduardo-elizondo · Pull Request #15892 · python/cpython (original) (raw)

Module-level constant strings should use PyUnicode_InternFromString rather than PyUnicode_FromString

Agreed, thanks for the fix!

I'd rather bring _Py_IDENTIFIER(fspath) back (for now) than reimplement _PyObject_LookupSpecial.

I think there's two options: 1) Solve this by creating a new C-API that handles this without a _Py_IDENTIFIER. Unfortunately, this will add yet another C-API function. 2) Not expand the C-API and fix the pattern at the extension level. There's only 2 C Extensions in the stdlib that use this pattern.
Given that this is not a very common pattern, I think it's best to go for approach # 2. I didn't modify anything just yet but I'd like to hear your thoughts!

A more systematic solution would be to implement another C-API to hide the abstraction there. That being said, I rather not expand the C-API given that this pattern is barely present in the stdlib. I've kept the change for now to

In the stdlib, I'd prefer using tp_free and tp_new directly rather than using the workarounds here.

Unfortunately, under PEP384, tp_free and tp_new are not public api so they can't be used 😞
Also, with the introduced changes, os.stat_result now fails. I will revert this change to be compliant with PEP384 and make all tests pass.

ScandirIterator_new and DirEntry_new are the same function.

Thanks for merging them!