`DFLinkedList` does not satisfy `input_iterator`

[ab9rf]

Our proxy for iterating the job list does not fully satisfy the C++ STL requirements to be an input_iterator which limits the ability to fully use this proxy type with more recent developements in C++. This should be addressed.

discord link

[ab9rf]

After poking at this for far too long the issue appears to be that DfLinkedList's iterator type is proxy while its const iterator type is I * &; as these types are not the same the template doesn't comport with the requirements of an input_iterator.

Looking through the history, this used to not be the case but @BenLubar changed it (commit 1606483e7e54c455deaacb1f617644e1460ddb16) to the current state of affairs as a "performance improvement". It seems to me that either backing this out or changing the const iterator to also use the proxy might yield compliance with the expectations of input_iterator, but I'm still sorting through the specifics.

[BenLubar]

Looks like I put proxy back in but only partially right after that commit: https://github.com/DFHack/dfhack/commit/b5eb541fd34c44c6268d9464f58d3392ffa8394e

It was so long ago I can't remember anything about what I was doing there.

[chdoc]

I'm starting to believe that whatever performance problems the proxy was trying to address, has become irrelevant in the meantime. In fact, changing the definition of struct DfLinkedList to be the empty struct, causes exactly two compilation errors: the two simple should-be-const iterations in suspendmanager.cpp, because I am a fan of ranged loops.

In light of this, I would say we can safely replace this with a naive implementation meeting the STL requirements.

[chdoc]

After some more analysis, I have come to the conclusion that the purpose of the proxy is to provide "proxy references" that, unlike naked I*&, implement an assignment operator that takes care of setting/unsetting the list_link fields of the item being placed/replaced:

https://github.com/DFHack/dfhack/commit/b5eb541fd34c44c6268d9464f58d3392ffa8394e#diff-fc625092d138aa24048ed147fb2c381fb05559c45890b82d9eb7ae399db32461R262-R276

I'm of two minds about this. On the one hand, this seems useful in maintaining the invariants of DFLinkedList. On the other hand, this might provide a false sense of security. Lists like df::global::world->jobs.list own the items they point to. Thus, assigning to a proxy object will leak memory, unless the caller assumes ownership of the object beforehand. Context Discord thread

For supporting range loops and std::ranges, proxies are of no use, because the proxy will be dereferenced before entering the body of the loop.

So the question is: what functionality should iterators over DFLinkedList actually have?

Note: there is also independent list editing support provided by MiscUtils.h. Unlike the editing functionality provided via iterators, this is actually used (e.g. in Job.cpp)