Audit the implementation for and remove dead code

Issue #461 resolved
Amir Kamil created an issue

In poking at various issues, I’ve run into a not-insignificant amount of dead code. For instance, PR 328 removed reflection.hpp and wait.hpp, which have been long obsolete. Other dead code I’ve encountered that is still in develop:

  • backend::during_user and backend::during_level
  • many #if 0'd out blocks of code, e.g. a 100+-line implementation of a hand-rolled barrier in barrier.cpp

I think this dead code poses maintenance problems. We should audit the implementation for dead code and remove it where appropriate.

Comments (5)

  1. Amir Kamil reporter

    Unused code in utility.hpp:

    • detail::nop_function
    • detail::constant_function
    • detail::trait_any
    • detail::trait_all
    • detail::add_lref_if_nonref
    • detail::add_clref_if_nonref
    • detail::add_rref_if_nonref
    • detail::decay_tupled (#if 0’d out)
    • detail::tuple_get_or_void (#if 0’d out)
    • detail::tuple_rvals (#if 0’d out)

  2. Dan Bonachea

    I agree that backend::during_user and backend::during_level are not only dead, but look incorrect (bad handling of the active_per argument) and thus actively dangerous if they were ever used, so should be removed.

    Similarly the hand-rolled barrier in barrier.cpp should definitely go.

    I'm far less worried about small, localized pieces of dead code in utility.hpp, assuming they look like utilities we don't happen to currently be using but might use again someday. Of course deleting them won't remove them from git history, but "out of sight, out of mind". However I don't object to #if 0 around such cases to mark them as currently unused.

    IMO other large #if 0'd blocks of dead code should be evaluated along similar lines on a case-by-case basis: if it's relatively localized and looks like something that could plausibly be useful some day, leave it. If it looks like it has major flaws or has been subsumed by a different block of code or pulls in significant otherwise-dead code elsewhere, then it's a candidate for removal. If you want to continue compiling the list we can discuss each.

  3. Amir Kamil reporter

    I’m inclined to remove the unused code in utility.hpp, leaving a note that they were removed after the 2021.3.0 release. That would give us a pointer to where to look if we need the code in the future. “Out of sight” is a feature, in my opinion.

    I’m fine with leaving small blocks of #if 0 code in.

  4. Amir Kamil reporter

    PR 344 implements my suggested changes.

    I didn’t do a complete audit of all the code. I did full audits of code in core headers (backend.hpp, backend_fwd.hpp, utility.hpp, future/fwd.hpp, future/future1.hpp) and spot checks elsewhere. I also tried gcov (both gcc and clang implementations) but was unable to get any useful information out of that.

  5. Log in to comment