bpo-1635741: sqlite3 now uses Py_NewRef and Py_XNewRef

[erlend-aasland]

Thanks for GH-23152, @vstinner! Fewer lines, improved readability: 7 files changed, 40 insertions, 87 deletions :)
Would you mind reviewing?

https://bugs.python.org/issue1635741

[erlend-aasland]

FYI, CI hangs at test_tag_bind (tkinter.test.test_ttk.test_widgets.TreeviewTest) ... Timeout (0:20:00)! Kick CI again?

[vstinner]

FYI, CI hangs at test_tag_bind (tkinter.test.test_ttk.test_widgets.TreeviewTest) ... Timeout (0:20:00)! Kick CI again?

You can ignore this known issue https://bugs.python.org/issue42142 The core dev who will merge your PR wil re-run the job.

[vstinner]

I like node->key = Py_NewRef(key);, it looks nice.

But I'm not sure about long lines like current_param = Py_XNewRef(PyDict_GetItemWithError(parameters, binding_name_obj)); which do many things at the same line:

• Call PyDict_GetItemWithError()
• Call Py_XNewRef()
• Assign current_param

[erlend-aasland]

But I'm not sure about long lines like current_param = Py_XNewRef(PyDict_GetItemWithError(parameters, binding_name_obj)); which do many things at the same line:

Fixed in 8528bf6

[Jongy]

Just created https://bugs.python.org/issue42287, then saw this PR, good work :) This Py_INCREF(x); y = x; hassle always annoyed me in CPython's sources.

I believe that such changes, when applied correctly (that is, if you did not change code semantics), should result in an identical binary. So a simple check to ensure you haven't broken anything would be to compare the final build before & after.

P.S make sure to strip debug info, because it does change (line numbers, ...)

@vstinner I think this PR (and future PRs regarding this conversion) do not require a news entry, right? because they do not introduce any user-facing change, only code enhancement and refactoring.

[erlend-aasland]

@vstinner Would you like further changes, or are you fine with this PR as it stands?

[vstinner]

LGTM.

[erlend-aasland]

@vstinner FYI, synced with master, ready for merge when you please.