This call (and the same one below) is the thing am the least confident about... Not clear to me whether we can call that from here, or if there is a better way to do the tagging?

Also not sure if this combination of static local wrapper + class function is the best way to do it? Avoids the need to define a specific struct as custom data parameter for BKE_library_foreach_ID_link call below...

utils because this also covers the static wrapper function below...

This is an (indirect) callback for BKE_library_foreach_ID_link, and as such those names are inspired from the naming in LibraryIDLinkCallbackData struct...

I don't mind adding comments about that if you really think this is needed (or if you have ideas for better names), but I think there is a limit where comments should not substitute to reading the code and general knowledge of Blender code too...

To be more clear, I don't there should be comments here explaining how callbacks for BKE_library_foreach_ID_link are working, and what kind of data they are getting?

TBH I'm much more annoyed/unsure about that combination of two functions (static wrapper callback + public method), as noted in a comment below. But I do not know a better way to do that right now (maybe std::bind could be used here actually...)

I totally disagree, your suggested code is much less clear to me about the underlying logic of this function. I would read it as 'by default, tag self for update and stop iteration, unless <some specific condition> is met, then keep looping'.

This is the exact opposite of the actual logic, which is `by default, keep looping without doing anything, unless <some specific condition> is met, then tag and return'.

If the double call to graph_id_tag_update annoys you, we could instead use a bool id_self_needs_tagging variable, and check this one at the end to decide if we can tag and break iteration, or not... But in that case I do not really see it as useful (as in, making things more readable and clear) either tbh.

I really doubt calling graph_id_tag_update_single_flag would be enough? graph_id_tag_update does much more than just finding the id_node and calling graph_id_tag_update_single_flag on it...

Note that the extra call to find_id_node in graph_id_tag_update is extremely unlikely to be done in large number, it would only happen once per ID, and only on those IDs that do need the tagging specifically because of internal depsgraph building decisions to remove/bring back some nodes, which only happens in a few specific cases and should not typically affect that many IDs.

As pointed out in commit message, this whole new check itself is extremely fast, almost all the extra processing time in deg building induced by this patch comes from the extra COW flushing it triggers.

Actually, last part about std::bind is stupid, C code will never know what to do with such exotic C++ object...

We agree on the general design rule I think, but to me those condition checks are integral parts of the 'core logic' of that function...

This function can be split up into two, since it now does two different things (its original functionality + this new pass over the nodes).

How would you call them then?

Because the original name is extremely generic and vague, so... would totally fit both functionalities.

I agree, the original name is super vague in terms of what the code actually does. How about something like tag_previously_tagged_nodes() and update_invalid_cow_pointers()?