Pull Request №706 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/core-empty-dtors-rule-of-zero
Merge: b2a0ff32f3ee01822f6f18b9d21ea27357a5f6bc←b9ffb8f38f6f8ae2fd2a072786cb3c81bfd4901f
Apply Rule of Zero to empty destructors across core/
----------------
Merge commit message:
Apply Rule of Zero to empty destructors across core/
Move trivially empty or `= default;`-bodied destructors from .cc files to
header declarations using `= default`, then remove the now-redundant .cc
implementations. Where a destructor was declared `~X() {}` inline in a
header, switch it to `~X() = default;` for the same reason.
Affected sibling groups:
- core/conformation/membrane/* and membrane_geometry/* (8 classes)
- core/conformation/parametric/*ValuedParameter (5 classes)
- core/io/silent/SilentFileData iterator and const_iterator
- core/pack/interaction_graph/RotamerDots family (4 classes)
- core/scoring/etable/EtableEvaluator hierarchy (3 classes)
- core/scoring/hbonds/graph/HBondInfo (2 classes)
- core/scoring/nmr/NMRDummySpinlabelVoxelGrid family (4 classes)
- core/scoring/sc/MolecularSurfaceCalculator::Atom
- core/energy_methods/SAXSEnergy
Pull Request №705 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/key-containers-rule-of-zero-redo
Merge: b2a0ff32f3ee01822f6f18b9d21ea27357a5f6bc←f7927e886605c9074a69feb917a084fac42262d8
Apply Rule of Zero to utility/keys container family (corrected)
----------------
Merge commit message:
Apply Rule of Zero to utility/keys container family
Applies Rule of Zero to five sibling key-container templates in
`source/src/utility/keys/`:
- `ClassKeyMap`
- `ClassKeyVector`
- `KeyVector`
- `SmallKeyMap`
- `SmallKeyVector`
Each held only standard-container value-type members (a `Vector`,
plus an `IndexMap` and a scalar `Index u_` in the `Small*` variants).
Their user-declared destructors (empty body or `= default`), copy
constructors, and copy-assignment operators were byte-for-byte
equivalent to the implicit defaults the compiler would synthesize,
so they were redundant.
Removing them lets the implicitly defaulted special-member-functions
take over and, as a side effect, restores the implicit move
constructor and move assignment that were previously suppressed by
the user-declared copy operations.
Explicit `= default` default constructors are kept on all five
classes. `SmallKeyMap` / `SmallKeyVector` need a user-defined default
constructor to value-initialize the scalar `Index u_`, which an
implicit default would leave indeterminate. `ClassKeyMap` /
`ClassKeyVector` / `KeyVector` need an explicit `= default` because
each has a user-declared (non-default) constructor (iterator-range
or size/value), and a class with any user-declared constructor does
not get an implicitly synthesized default. The previous attempt at
this refactor (#691, reverted by #703) dropped these `= default`
defaults on the assumption that they would be synthesized
implicitly, which broke `ClassKeyMap m;` in test/utility/keys/
ClassKeyMap.cxxtest.hh. Keeping the explicit defaults preserves the
original API while still letting copy / move / destructor be
implicit. Supersedes #704.
Pull Request №701 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/fix/audit-comparison-and-off-by-one
Merge: dc594471462f126ea60fc1148900ed66e5b77e44←b2883867f98b1a2e01669b1ea8b4321551565904
Fix correctness bugs: broken operator< and off-by-one residue loops
----------------
Merge commit message:
Fix correctness bugs: broken operator< and off-by-one residue loops
Two unrelated families of correctness bugs found by an audit pass.
Continues the patterns from #696 (broken comparison operators) and
#695 (off-by-one residue loops).
Broken operator< implementations (violate strict weak ordering):
* core/environment/LocalPosition.cc: assigned the signed result of
std::string::compare() to the unsigned alias Size, so any
"this->label_ < other.label_" case wrapped to a huge value and the
cmp > 0 branch fired. On top of that, the cmp > 0 branch itself
was inverted relative to "less than". Combined effect: for any
two LocalPositions with distinct labels, both a < b and b < a
returned true. The class is used as the key of
std::map<LocalPosition, core::Real> in CutBiasClaim. Replaced
with std::tie lexicographic compare.
* protocols/environment/claims/ClaimStrength.cc: every comparison
operand was reversed, so operator<(*this, other) returned true
exactly when *this was lexicographically greater than other --
the operator was fully inverted relative to its name and to
operator==. Replaced with std::tie lexicographic compare.
Off-by-one loops over 1-indexed residue / vector1 ranges:
* protocols/simple_moves/ExplicitWaterMover.cc get_water_recovery:
four loops using i < pose.total_residue() (and i < vec.size()
on utility::vector1<>, which is 1-indexed). The last residue and
last vector element were skipped, so water-recovery counts and
the per-water coordinate lists silently dropped the final entry.
* protocols/ligand_docking/GALigandDock/GALigandDock.cc
calculate_free_ligand_score: the loop adding CoordinateConstraints
to anchor every atom used ires < pose->total_residue(), so the
last residue's atoms were not constrained.
* protocols/membrane_benchmark/MembraneEnergyLandscapeSampler.cc
count_res / count_diff: both helpers walked
ii < pose.total_residue(), so a matching last residue was never
counted and a differing last residue's mismatch was never
reported.
Pull Request №700 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/core-private-unimpl-copy-modernization
Merge: dc594471462f126ea60fc1148900ed66e5b77e44←ff37fbbb56e33fdf9b332f6757faaef2bb50e2c6
Modernize pre-C++11 copy prevention to = delete across core/ and protocols/
----------------
Merge commit message:
Modernize pre-C++11 copy prevention to = delete across core/ and protocols/
Replace the pre-C++11 idiom (private undefined copy ctor / copy assignment to
prevent copies via link error) with explicit `= delete` for clearer diagnostics
and compile-time enforcement. 16 files; no behaviour change.
Two flavours:
1. Stand-alone non-copyable classes: replace the unimplemented private
declarations with `= delete`. Drops redundant comments ("deny use of the
copy constructor", "private and unimplemented", etc.) that are now
self-evident from `= delete`. Keeps explanatory comments where the *why*
is non-obvious (ScoreFunction / DockingScoreFunction / OtherContextScoreFunction:
"copying discards subclass information; use clone() instead").
2. Singleton (SymDofMoverSampler, inheriting utility::SingletonBase): drop
the redundant copy/assignment declarations entirely — the base already
`= delete`s its own copies, which transitively makes the derived
implicit copies deleted.
Files: ScoreFunction, MinScoreScoreFunction, DockingScoreFunction,
OtherContextScoreFunction, RotamerSet, RotamerSet_, RotamerSubset, RotamerSetBase,
DofPassport, ConstantLengthFragSetIterator_, FrameListIterator_,
MinimalFragSetIterator_, OrderedFragSetIterator_, FragmentPicker (QuotaDebug),
SymDofMoverSampler, FileSystemResourceLocator (FileStream).
Debug build passes clean.
Pull Request №699 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/lr-energy-container-iterators-rule-of-zero
Merge: 708ccac488292139132ec4853814381fb0bdd6ae←4bf6498bfe1e1ad5d8520a2090d1c5deb6823496
Apply Rule of Zero to remaining LREnergyContainer iterator subclasses
----------------
Merge commit message:
Apply Rule of Zero to remaining LREnergyContainer iterator subclasses
Extends the disulfide-iterator pattern (PR #689) to the four remaining
ResidueNeighbor{,Const}Iterator subclasses:
- DenseNeighbor{,Const}Iterator
- OneToAllNeighbor{,Const}Iterator
- PolymerBondedNeighbor{,Const}Iterator
- CstResNeighb{,Const}Iterator
For each: drop the empty `= default` destructor pair (override decl in .hh,
out-of-line definition in .cc), and replace the pre-C++11 private undefined
copy-assignment with an explicit `= delete` plus a comment explaining that
all assignment must funnel through the polymorphic operator= so derived
state is downcast and copied correctly.
This exhausts the LREnergyContainer iterator family; together with PR #689
all six concrete container's iterator pairs use the same idiom.
Pull Request №697 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/rule-of-zero-batched-fixes
Merge: d10f3af574b60550f256856d5a9742600ce1128b←5d92187e14c53fcb0249c5ee8f5426908f3a44e3
Apply Rule of Zero to old-style uncopyable classes in core/ and protocols/
----------------
Merge commit message:
Remove dead private fields exposed by = delete copy modernization
Clang's -Wunused-private-field is suppressed while a class has any
declared (even unimplemented) private copy constructor / assignment,
because the compiler can't prove those bodies don't touch the field.
Replacing those declarations with = delete in PR #697 let clang fully
analyze member usage, surfacing five orphaned scalar fields whose only
references live inside large /* ... */ blocks of disabled I/O code:
- DoubleLazyNode::procrastinated_
- LazyNode::procrastinated_
- LazyInteractionGraph::num_aa_types_
- PDInteractionGraph::num_nodes_in_file_
- PDInteractionGraph::num_file_aa_types_
Drop the fields and the matching constructor initializers. The commented
I/O functions that reference them are left alone (already dead).
Pull Request №697 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/refactor/rule-of-zero-batched-fixes
Merge: d10f3af574b60550f256856d5a9742600ce1128b←4d2c33ca7a4b5531b084f002cd469d790f5fa119
Apply Rule of Zero to old-style uncopyable classes in core/ and protocols/
----------------
Merge commit message:
Apply Rule of Zero to old-style uncopyable classes in core/ and protocols/
Modernize 31 files that used the pre-C++11 idiom of declaring (but not
defining) private copy constructor and assignment operator to forbid
copies. The link-error-as-enforcement trick predates `= delete` and is
fragile (silent same-translation-unit ICE breakage, confusing diagnostics).
Two flavours of fix:
1. Stand-alone non-copyable classes (InteractionGraphBase node/edge/graph
family, FlexbbInteractionGraph, JobDigraph, AtomTreeCollection's
ResidueAtomTreeCollection, Matcher, MonteCarlo's operator=): replace
the unimplemented private declarations with public `= delete` copies.
Where the old assignment took a non-const reference or returned `T const &`
that was a holdover from the C++03 idiom; the modernized signatures take
`T const &` and return `T &`.
2. Singleton-derived factories (utility::SingletonBase children): drop the
redundant unimplemented copy/assignment declarations entirely. The base
class already `= delete`s its own copy/assignment, which transitively
makes the derived class's implicit copies deleted, so the extra lines
were noise.
No behaviour change: copies/assignments that were link errors before are
now compile errors, which is the intended improvement in diagnostic
quality. Debug build passes clean.
Pull Request №696 RosettaCommons/rosetta/main ← lyskov-ai/rosetta/fix/incorrect-comparison-operators
Merge: d10f3af574b60550f256856d5a9742600ce1128b←453f38ad4e9ffd93c2be2fcabe2843fe3a16c9a8
Fix incorrect comparison logic in operator< / difference_from
----------------
Merge commit message:
Add comments explaining each comparison-operator fix
Brief one-paragraph notes above each of the four sites describing
the bug and why the corrected form is the right one, so a future
reader doesn't accidentally revert to the simpler-looking but
broken version.