GeezerVison

Small, easy to miss point here: passing source=coro here means that if the user happens to have tracemalloc enabled, its origin traceback will be included in the warning. Kinda neat! It does mean if you have both set_coroutine_origin_tracking_depth and tracemalloc enabled you'll get two at-least-partially-redundant tracebacks in the same warning. I don't know that I care that much about this but FYI.

I think a redundant warning is OK. tracemalloc is enabled quite rarely. I also think what you wrote in this comment should be reflected in a code comment in case someone debugs the double warnings issue :)

thread-local -> thread-specific

Can you add a link to the set_coroutine_origin_tracking_depth documentation snippet?

Well, that was my original thought. The problem is that

:func:`set_coroutine_origin_tracking_depth`

is too long to fit in the ascii-art box. So... either we need a shorter name for the function, or we need to redraw this whole giant table, and I couldn't think of a satisfactory way to do either in the 2 minutes I spent thinking about it :-). Any suggestions?

I guess we could use that weird ReST substitution thing? I'm not sure how that works.

Does this work: http://docutils.sourceforge.net/docs/user/rst/quickref.html#hyperlink-targets ?

Need to specify how the list is ordered.

Strictly speaking we don't need PyAPI_FUNC here, as warnings.c|h will be linked with genobject.c and we are not going to use it in extensions.

if enabled and self._coroutine_origin_tracking_enabled:?

I don't like using == for bools.

No, that would be different – the no-== equivalent would be:

if ((enabled and self._coroutine_origin_tracking_enabled) or (not enabled and not self._coroutine_origin_tracking_enabled)):
   ...

I find the == version easier to read. ("If the requested state equals the current state, return without doing anything.")

Got it, then I'd cast them both to bool: if bool(enabled) == bool(self._coroutine_origin_tracking_enabled):

Again, per PEP 7:

if ( ... ) {
}
else {
}

Also I'd like this whole else branch to be in a separate helper function.

The idea is neat, but it makes the code harder to read and it's not how we usually do it in CPython. While reviewing the code I've already added 2 comments asking you to add Py_DECREF(origin). Please do it the long way: create, populate, cleanup on errors, and finally assign it to cr_origin. After all it's just two places where an extra decref is needed.

Can we call PyList_SetSlice only when needed?

Just add Py_DECREF(fn) after this line.

Add assert(new_depth >= 0) for cases when somebody uses the C API directly (sys.set_coroutine_origin_tracking_depth has a check for Python users).

I just realized that exposing list object directly through cr_origin means that users can mutate it. How about we cycle through the frames two times: 1st time to determine the real depth we can capture; 2nd time to populate a cr_origin tuple (not list). Traversing a relatively short chain of frames should be pretty fast, and this is a debug mode after all. What do you think?

Another option would be to keep things as is, but to change cr_origin to a getter, and return a copy of the list every time. But I like the tuple idea more.

Right, I originally left out the Py_VISIT call and then realized that mutation was possible, so figured I'd put it in just in case :-).

I don't think it matters too much whether we expose a tuple, copy the list, or just keep the current thing where someone can mutate it if they really want to. (The warnings code is robust against such shenanigans: worst case it'll print an error b/c cr_origin was corrupt, and then the real warning without a traceback.) Given this I have a mild preference not to do the tuple thing, because it leads to the most complicated C code. Doing an extra loop is totally doable of course, but it's also plenty complicated to hide some kind of off-by-one bug or something.

Add sys.get_coroutine_origin_tracking_depth?

Eh, I guess. I was thinking this is sufficiently arcane functionality that I could be lazy and get away with the set-returns-the-old-value pattern (cf. signal.set_wakeup_fd), but you're probably right that set+get is a nicer API.