Skip to content

Apply Rule of Zero across remaining utility/ empty destructors#694

Open
lyskov-ai wants to merge 1 commit into
RosettaCommons:mainfrom
lyskov-ai:refactor/utility-misc-empty-dtors
Open

Apply Rule of Zero across remaining utility/ empty destructors#694
lyskov-ai wants to merge 1 commit into
RosettaCommons:mainfrom
lyskov-ai:refactor/utility-misc-empty-dtors

Conversation

@lyskov-ai
Copy link
Copy Markdown
Contributor

Summary

Bundle of small Rule-of-Zero / clarity fixes across utility/ for classes
not covered by any other open PR. All changes are observably no-ops at
runtime (each destructor body either was empty or = default); the goal
is to remove redundant declarations and document a deliberate non-trivial
case.

  • Remove empty ~Foo() {} where the implicit destructor is already
    correct (virtual is preserved via the base class when relevant):
    Bound, Exception, ocstream, AutoKey, UserKey.
  • Convert ~Foo() {} to ~Foo() = default; for polymorphic root
    classes (no virtual base destructor to inherit), so the destructor
    stays virtual: Show, WidgetFactory, irstream, orstream, Key,
    Option.
  • Drop matching = default destructor pairs (.hh decl + .cc def) for
    classes that inherit from utility::VirtualBase, which already
    supplies a virtual ~VirtualBase() = default;: heap,
    subset_mapping, recent_history_queue, GeneralFileContents,
    GeneralFileContentsVector, Tag.
  • utility/io/mpistream.hh: the destructor of basic_mpi_streambuf
    is not trivial — it calls flush_final(), which sends the close
    message on the MPI channel. Add explicit = delete for its copy
    constructor and copy-assignment so an accidental copy can't trigger
    the side effect twice. Also replace the empty
    ~basic_mpi_ostream() override {} with = default.

The diff is intentionally smaller than the usual 128-line bundle target:
most other empty-destructor candidates in utility/ are already covered
by my open sibling PRs (#690 BitSet/BitVector, #691 keys containers, #692
utility/ helpers, #693 utility/options empty dtors). This PR sweeps the
classes those don't touch.

Verified by a clean mode=debug build (python3 ./scons.py -j16 mode=debug,
zero errors, no new warnings).

Remove user-declared destructors and out-of-line `= default` definitions
where the implicit (or `= default`) destructor is correct, and clarify a
non-trivial destructor by deleting copy/move on its owner.

* Remove empty `~Foo() {}` / `~Foo() override {}` where the implicit
  destructor is already correct (and virtual via base when needed):
    - utility/Bound.hh, utility/excn/Exceptions.hh,
      utility/io/ocstream.hh, utility/keys/AutoKey.hh,
      utility/keys/UserKey.hh.

* Convert `~Foo() {}` to `~Foo() = default;` for polymorphic root
  classes whose implicit destructor would otherwise be non-virtual:
    - utility/Show.hh, utility/factory/WidgetFactory.hh,
      utility/io/irstream.hh, utility/io/orstream.hh,
      utility/keys/Key.hh, utility/options/Option.hh.

* Drop `= default` destructor pairs (.hh declaration + .cc definition)
  for classes inheriting from utility::VirtualBase, which already
  supplies a virtual destructor:
    - utility/heap, utility/integer_mapping (subset_mapping),
      utility/recent_history_queue, utility/io/GeneralFileManager
      (GeneralFileContents{,Vector}), utility/tag/Tag.

* utility/io/mpistream.hh: explicitly `= delete` the copy ctor and
  copy assignment of `basic_mpi_streambuf` because its destructor
  calls `flush_final()` (closes the MPI channel); replace the empty
  `~basic_mpi_ostream() override {}` with `= default`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants