gh-112066: Add PyDict_SetDefaultRef function.

[colesbury]

The PyDict_SetDefaultRef function is similar to PyDict_SetDefault, but returns a strong reference through the optional **result pointer instead of a borrowed reference.

• Issue: Provide a variant of PyDict_SetDefault that returns a new reference (instead of a borrowed reference) #112066

---

📚 Documentation preview 📚: https://cpython-previews--112123.org.readthedocs.build/

[colesbury]

cc @zooba, @encukou, and @serhiy-storchaka in case you are interested in this

[zooba]

My only comment is the same as for the recently added pop functions: we should not use these functions as precedent for their API design (specifically how the return value is handled). When we come to decide which approach we want to standardise for the C API in general, we aren't going to be forced into choosing this design just because it was used during the period between deciding-to-decide and actually deciding.

But since there's a chance that the other recently added API could also become "warts", best to align with those since they're all on the same type. There's nothing gained by deliberate inconsistency when the functions show up right next to each other.

[serhiy-storchaka]

Very nice. The code looks good.

But I worry that all exit branches simply add Py_NewRef() for PyDict_SetDefaultRef(). It means that PyDict_SetDefaultRef() is not principally different from PyDict_SetDefault() combined with Py_XNewRef(). The question arises about the need to add a new function.

Could you please create a separate PR that replaces all PyDict_SetDefault() with PyDict_SetDefaultRef() to show what benefit it could bring.

[encukou]

we should not use these functions as precedent for their API design

IMO, if the C-API WG is formed and decides soon enough, this API and all its uses in CPython might be adjusted. That might affect in-flight PRs.

---

PyDict_SetDefaultRef() is not principally different from PyDict_SetDefault() combined with Py_XNewRef()

With nogil, the latter is incorrect. The reference can become invalid (and the object deallocated) between the PyDict_SetDefault() and Py_XNewRef() calls.
(That doesn't apply if you know no other threads can mutate the dict, but future changes can easily invalidate assumptions like that.)

[colesbury]

As @encukou wrote, returning a strong reference is important in nogil builds and can't be safely simulated with a subsequent Py_INCREF().

I'll put up a subsequent PR that replaces internal PyDict_SetDefault calls with PyDict_SetDefaultRef.

[colesbury]

@serhiy-storchaka - here is an example PR that changes usages: #112211. When this PR lands, I'll rebase that PR on top of it.

[colesbury]

Are there any outstanding concerns with this change @encukou @serhiy-storchaka?

[serhiy-storchaka]

Thank you for #112211 (yet one nice number!).

Please add a What's New entry. The rest LGTM.

[colesbury]

I've added the "What's New" entry

[serhiy-storchaka]

LGTM.

[colesbury]

@serhiy-storchaka - would you please merge this?

[serhiy-storchaka]

Provided for consideration of the C API WG: capi-workgroup/decisions#4.

[colesbury]

Looks like the C API WG approved the API. @serhiy-storchaka, can we merge this PR now?

[serhiy-storchaka]

One more issue needs to be resolved. Wait another week. 😉

[serhiy-storchaka]

The issue I was talking about was the ongoing vote to promote @colesbury as a core developer. I think it would be nice if Sam merge his PR himself. Welcome on the board!