Feature request: Better TreeNode management

[VictorYing]

I'm thinking about working on a replacement for the std::vector<std::unique_ptr<sparta::TreeNode>> to_delete_; member inside of the sparta::app::Simulation base class. I don't like the existing mechanism because modelers who have requirements that the destructor of one TreeNode is called before the destructor of another TreeNode seem to be getting the habit of shuffling their TreeNodes into this vector into a particular order, and then relying on the fact that GNU libstdc++'s std::~vector() happens to destruct its elements from front to back. The example code in this repo already demonstrates this fragile pattern:

map/sparta/example/DynamicModelPipeline/src/ExampleSimulation.cpp

Line 693 in e736f0d

to_delete_.insert(to_delete_.begin(), std::unique_ptr<sparta::TreeNode>(rtn));

I have two reasons not to like this pattern:

• I would prefer to have a default where nodes are destructed in the reverse order they are constructed. This is because I feel is natural to write code that constructs parent nodes before children, and to place some shared resource in the parent (e.g., a SpartaSharedPointerAllocator) that children may depend on (e.g., the children may hold shared pointer instances that need to get destructed before the allocator is destructed).
• The C++ language standards leave the order of destruction within most STL containers unspecified: https://stackoverflow.com/questions/6169125/order-of-destruction-of-elements-of-an-stdvector
  Models that work on GNU libstdc++ may break when linked against LLVM's libc++, where std::vector destructs elements back-to-front (consistent with the behavior the C++ standard imposes for raw arrays): https://godbolt.org/z/h3Pon4oT9

I suggest keeping to_delete_; (there's too much model code out there that uses it) while introducing a replacement API that registers TreeNodes to be managed by sparta::app::Simulation base class by storing them in some private member and ensuring the destructor destructs them in a well-specified order. Maybe in MAPv3 we can formally deprecate to_delete_ but I wonder if it's okay to go ahead and introduce a new solution now within the MAPv2 series.

[furuame]

I think it's reasonable to define the deallocation order of to_delete_ to avoid unspecified behavior that you described. We could explicitly clean up to_delete_ at Simulation's destructor.

On the other hand, I'm assuming you're building a custom model by DynamicModelPipeline. Defining another set of TreeNodes and the desired destruction behavior in your derived Simulator class might also work out. Just replace the strings :)