Apply Rule of Zero across remaining utility/ empty destructors (#694)
## 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`.
Apply Rule of Zero across utility/options/ empty destructors (#693)
## Summary
Drop user-declared empty destructors from the `utility/options/` and
`utility/options/keys/` class hierarchies. The implicit destructor
preserves virtual dispatch in every case via inherited virtual
destructors:
- All option classes inherit (transitively) from
`utility::options::Option`, which declares `virtual ~Option()`.
- All option-key classes inherit (transitively) from
`utility::keys::Key`, which declares `virtual ~Key()`.
No class touched here owns a raw resource, declares a non-trivial
destructor body, or has a user-declared copy/move that would suppress
the implicit destructor.
### `utility/options/` — abstract bases and remaining leaf option
classes
* Abstract / templated bases: `ScalarOption`, `ScalarOption_T_`,
`VectorOption`, `VectorOption_T_`, `AnyOption`, `AnyVectorOption`.
* Leaf options: `PathOption`, `PathVectorOption`, `StringOption`,
`StringVectorOption`, `ResidueChainVectorOption`.
`AnyOption` / `AnyVectorOption` used the `virtual` keyword on the empty
body; the rest used `override {}`. Both forms are equivalent to the
implicit virtual destructor here.
### `utility/options/keys/` — base + 16 leaf key types
* Base: `OptionKey`.
* Leaves: `AnyOptionKey`, `AnyVectorOptionKey`, `BooleanOptionKey`,
`BooleanVectorOptionKey`, `FileOptionKey`, `FileVectorOptionKey`,
`IntegerOptionKey`, `IntegerVectorOptionKey`, `PathOptionKey`,
`PathVectorOptionKey`, `RealOptionKey`, `RealVectorOptionKey`,
`ResidueChainVectorOptionKey`, `ScalarOptionKey`, `StringOptionKey`,
`StringVectorOptionKey`, `VectorOptionKey`.
This is the natural follow-up to #692, which intentionally deferred this
batch. With this PR, the entire `utility/options*`
empty-virtual-destructor pattern is cleaned up.
29 files, 156 deletions, 0 additions. Debug build clean.
Apply Rule of Zero across utility/ helpers (#692)
## Summary
Remove user-declared destructors and trivial copy ctor/assignment
implementations whose bodies are equivalent to the compiler-generated
defaults. The implicit special members are correct in every case (no
owning raw pointers, no side effects in the removed bodies, virtual
destructor inheritance preserved through base classes).
### utility/graph/ — non-owning iterator/element helpers
* `Graph.hh`: `EdgeListElement`, `EdgeListIterator`,
`EdgeListConstIterator` — drop user dtor and the `= default` copy ctor /
assignment.
* `Digraph.hh`: `DirectedEdgeListElement`, `DirectedEdgeListIterator`,
`DirectedEdgeListConstIterator` — drop user dtor and member-wise copy
ctor / assignment.
* `ArrayPool.hh` (`Array0`): drop user dtor, copy ctor, and
self-assignment-guarded copy assignment.
* `LowMemGraph.hh` (`LowMemGraphBase`): drop empty `override` dtor
(`utility::VirtualBase` supplies the virtual destructor).
* `UpperEdgeGraph.hh`: drop the empty `UEEdge` dtor and
`~UpperEdgeGraph() override = default;`.
`LowMemNode` / `LowMemEdge` are intentionally left untouched — their
explicit destructor declarations anchor a load-bearing comment about
deliberately not making the destructor virtual.
### utility/
* `DereferenceIterator.hh` (`DereferenceIterator`): drop empty user
dtor.
### utility/options/ — empty overriding destructors
Drop `~XxxOption() override {}` from the eight leaf scalar/vector option
classes. The implicit destructor is virtual via inheritance from
`Option` (which declares a virtual destructor), so dynamic dispatch is
preserved.
* `BooleanOption.hh`, `BooleanVectorOption.hh`
* `FileOption.hh`, `FileVectorOption.hh`
* `IntegerOption.hh`, `IntegerVectorOption.hh`
* `RealOption.hh`, `RealVectorOption.hh`
The remaining option classes with the same pattern (`PathOption`,
`StringOption`, `ResidueChainVectorOption`, `ScalarOption`,
`ScalarOption_T_`, `VectorOption`, `VectorOption_T_`) can be addressed
in a follow-up; the eight here form a coherent leaf-scalar-plus-vector
subset.
Improving `Pose.cache` dictionary getter and setter performance (#658)
This PR aims to improve the performance of `Pose.cache` dictionary data
accessors. Several code pathways run with O(N^2) (quadratic time
complexity) behavior, and new functionally equivalent fast data accessor
methods are introduced to run with O(N) (linear time complexity)
behavior:
- `Pose.cache.fast_items()`
- `Pose.cache.fast_values()`
- `Pose.cache.metrics.fast_items()`
- `Pose.cache.metrics.fast_values()`
- `Pose.cache.metrics.real.fast_items()`
- `Pose.cache.metrics.string.fast_values()`
- `Pose.cache.metrics.composite_real.fast_items()`
- `Pose.cache.metrics.composite_real.fast_values()`
- `Pose.cache.metrics.composite_string.fast_items()`
- `Pose.cache.metrics.composite_string.fast_values()`
- `Pose.cache.metrics.per_residue_real.fast_items()`
- `Pose.cache.metrics.per_residue_real.fast_values()`
- `Pose.cache.metrics.per_residue_string.fast_items()`
- `Pose.cache.metrics.per_residue_string.fast_values()`
- `Pose.cache.metrics.per_residue_probabilities.fast_items()`
- `Pose.cache.metrics.per_residue_probabilities.fast_values()`
- `Pose.cache.extra.fast_items()`
- `Pose.cache.extra.fast_values()`
- `Pose.cache.extra.real.fast_items()`
- `Pose.cache.extra.real.fast_values()`
- `Pose.cache.extra.string.fast_items()`
- `Pose.cache.extra.string.fast_values()`
- `Pose.cache.energies.fast_items()`
- `Pose.cache.energies.fast_values()`
Users must update their API calls to take advantage of these upgrades:
`dict(pose.cache)` -> `dict(pose.cache.fast_items())`, etc. These
improvements are only really noticable when there are hundreds to
thousands of scores cached in the `Pose.cache` dictionary. The basis for
the performance improvement is the following:
- `dict(pose.cache)` relies on `__iter__` (returns `pose.cache.all`) +
`__getitem__(key)` (returns `maybe_decode(pose.cache.all[key])`), where
it materializes the full scores dictionary for each key (O(N^2)).
- Instead, `dict(pose.cache.fast_items())` relies on simply `for k, v in
pose.cache.all.items(); yield k, maybe_decode(v)`, so the full scores
dictionary is materialized once for all keys (O(N)).
- It's also worth noting that the deprecated `Pose.scores` dictionary
(note `scores` not `cache`) has always performed with quadratic time
complexity (O(N^2)), and does not contain `Pose.scores.fast_items()` or
`Pose.scores.fast_values()` methods.
This PR also makes the `Pose.cache.all_scores` property run with O(N)
behavior, and removes an unnecessary argument from a private method:
`self._has_sm_data(pose)` -> `self._has_sm_data()`.
Additionally, this PR provides two new fast setter methods for mappables
(avoiding the relatively slow `Pose.cache.metrics` cleanup after each
item is set with `__setitem__`, and instead only performing one cleanup
at the end):
- `Pose.cache.metrics.real.set_mappable()`
- `Pose.cache.metrics.string.set_mappable()`
Micro-updates to the `PyRosettaCluster` interface are made to take
advantage of these performance improvements.
Apply Rule of Zero to BitSet and BitVector (#690)
- `utility::BitSet<B>` and `utility::BitVector<B>` each had only a
single value-type member (`std::set<B>` and `std::vector<bool>`
respectively), no user-declared copy/move/assignment operators, and an
explicit empty destructor.
- The empty `~BitSet() {}` / `~BitVector() {}` did nothing the
compiler-generated destructor wouldn't have done, and they suppressed
the implicit move constructor and move assignment.
- Remove the empty destructors so both class templates follow the Rule
of Zero and gain implicit move support.
Apply Rule of Zero to disulfide energy neighbor iterators (#689)
- Remove user-declared trivial (defaulted) destructors from the six
disulfide neighbor iterator classes; the implicit virtual destructor
inherited from `ResidueNeighborIterator` /
`ResidueNeighborConstIterator` does the same job.
- Modernize the old-style C++03 copy-assignment prevention (private
undefined declaration) to a public `= delete` for the derived-type
`operator=`. Behavior is unchanged: derived-derived assignment is still
forbidden so all assignments flow through the polymorphic
`operator=(BaseType const &)` override that downcasts and copies all
members.
Apply Rule of Zero to core signal event hierarchies (#688)
Each of the simple polymorphic signal-event classes in
`core/conformation/signals/` and `core/pose/signals/` had a hand-written
copy constructor, copy assignment, and empty destructor that all just
did what the compiler-generated versions would do. Removing the
boilerplate brings them in line with the Rule of Zero while preserving
the polymorphic base destructors as `= default`.
Touched:
- `core/conformation/signals/`: `GeneralEvent`, `ConnectionEvent`,
`IdentityEvent`, `XYZEvent`
- `core/pose/signals/`: `GeneralEvent`, `DestructionEvent`,
`ConformationEvent`, `EnergyEvent`
`LengthEvent` is intentionally left alone — its copy constructor and
copy assignment invoke `check_consistency()` in debug builds, so they
have real semantic content beyond member-wise copy.
Modernize copy prevention in core factory singletons and non-copyable classes (#685)
- **SilentStructFactory, ConstraintFactory, SequenceFactory**: all
inherit from `utility::SingletonBase`, which already `= delete`s copy
construction and copy assignment. Removes the now-redundant
private-but-unimplemented declarations from each derived class
(pre-C++11 idiom).
- **OrbitalsStatistics, AtomTreeDiff**: had private-but-undefined copy
constructors (pre-C++11 idiom); replaced with explicit `= delete` on
both copy constructor and copy assignment operator.
No behaviour change; build-only verification is sufficient for pure
copy-prevention modernisation.
Modernize copy prevention in Emitter, YamlEmitter, and JsonEmitter (#678)
- `Emitter`, `YamlEmitter`, `JsonEmitter`: replace private/protected
undefined copy ctor declarations with explicit `= delete` in the public
section, and add the missing copy assignment `= delete`.
- Also remove the redundant undefined null-ctor declarations in
`YamlEmitter` and `JsonEmitter` — these were suppressed automatically in
C++11 by the presence of user-declared non-default constructors.
Modernize copy prevention in basic_otstream and TracerImpl (#674)
- `basic_otstream`: replace private-undefined copy ctor with explicit `=
delete` for both copy ctor and copy assignment in the public section.
Adds a one-liner comment explaining the destructor side-effect (`delete
this->rdbuf()` causes double-free if two objects share the same buffer).
- `TracerImpl`: move the already-`= delete`d copy ctor from private
section to public, and add the missing copy assignment `= delete`.
Part of the ongoing Rule-of-Zero / modern C++ idiom cleanup in `basic/`.
Modernize copy prevention in basic/ComparingTracer and MessageListenerFactory (#673)
Two related cleanups in `basic/`:
**`ComparingTracer.hh`**: Replace private-and-undefined copy constructor
with public `= delete`; also explicitly delete copy assignment. The
`std::fstream` member already makes copies impossible, but `= delete`
documents the intent at the class level.
**`mpi/MessageListenerFactory.hh`**: Remove the redundant
private-undefined copy constructor and copy assignment.
`MessageListenerFactory` inherits from
`utility::SingletonBase<MessageListenerFactory>`, which already deletes
both copy operations — the manual declarations were dead code.