Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-280

Remove or disable daf::base::Citizen


    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM
    • Labels:


      Citizen class is a C++ base class that is designed to help debug memory leaks. Its SDSS predecessor (C, not C++) was apparently incredibly useful, but with our use of smart pointers and RAII, I believe it's essentially been a non-issue. I can't recall a single instance in which it was helpful in debugging a memory problem, and it's the source of a constant trickle of confusion due to failures to call Citizen::markPersistent and the inherited getId method looking like it returns some other kind of ID.

      I think we should at least #define out the Citizen implementation by default. That'll make it truly a zero-cost utility in non-debug builds, and let us use it in more places (right now, we don't use it for small classes, which decreases its utility even further). If we do that, I'd also advocate changing the interface to only provide access via a separate class (see DM-675).

      But I think we could save the most effort by just removing it entirely, as that'll save on future maintenance effort (such as modifying the threading stuff to use C++11 constructs, or just making sure it works properly with pybind11, which is pickier than Swig about returning raw pointers). I'd like to specifically ping Robert Lupton on that question (as well as the broader usual audience for RFCs) - I know he agrees that Citizen isn't as useful as it was in C, but I'm very interested in his thoughts about removing it entirely.

      I'd also be interested to hear from people coming from other C++ projects as to whether they used (or wish they'd had) similar classes.


          Issue Links



              • Assignee:
                jbosch Jim Bosch
                jbosch Jim Bosch
                Gregory Dubois-Felsmann, Jim Bosch, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Paul Price, Pim Schellart [X] (Inactive), Robert Lupton, Russell Owen, Tim Jenness
              • Votes:
                1 Vote for this issue
                10 Start watching this issue


                • Created:
                  Planned End:

                  Summary Panel