Skip to content

Fix GCC 16 (trunk) debug build: real bug + dead unused-but-set-variable cleanup - #728

Merged
lyskov merged 4 commits into
RosettaCommons:mainfrom
lyskov-ai:fix/gcc16-debug-build
Jul 9, 2026
Merged

Fix GCC 16 (trunk) debug build: real bug + dead unused-but-set-variable cleanup#728
lyskov merged 4 commits into
RosettaCommons:mainfrom
lyskov-ai:fix/gcc16-debug-build

Conversation

@lyskov-ai

Copy link
Copy Markdown
Contributor

Summary

This machine has GCC 12 through GCC 16 (16 is an experimental trunk build) installed side by side. A survey of the debug library build under each version found:

Building under GCC 16 (with #723's fix applied) surfaced 16 unique -Werror sites — mostly -Wunused-but-set-variable, plus two -Wmaybe-uninitialized cases (one of which is a real bug):

  1. Real bugcore/chemical/CacheableResidueTypeSets.cc: the copy constructor initialized its base class with CacheableData(*this) instead of CacheableData(other), reading from the not-yet-constructed destination object rather than the fully-constructed source. Harmless today only because CacheableData has no data members of its own; still wrong and exactly what GCC 16 is right to flag.
  2. False-positive triggerprotocols/simple_moves/MissingDensityToJumpMover.cc: the default constructor called MissingDensityToJumpMover::get_name() (a qualified call through *this, mid-construction) to build an argument for the Mover base class. get_name() just returns a string literal, so it's passed directly instead — avoids the pattern rather than working around a compiler quirk.
  3. Dead loop counters (13 sites across 9 files) — variables incremented alongside a real loop iterator but never read anywhere: EnergyGraph.hh (iilag, 2 of 4 occurrences — the other two are real array indices and are untouched), PDBInfo.cc (idx, x2), mmtf_writer.cc (chainIndex, modelIndex), md.cc (imap), StructureDataFactory.cc (cur_chain), FoldArchitectMover.cc (count), pose_mod.hh (current_pos), DistanceScoreMover.cc (ct_peaks), StructureDependentPeakCalibrator.cc (pose_ct). No behavior change — removed the tracking, left the actual iteration logic untouched.
  4. Deliberately-unused, keptSapConstraintHelper.cc's offset is tracked "for symmetry" per an existing comment even though never read. Rather than removing it against that stated intent, added an explicit (void)offset; cast to satisfy the warning.

Verified with a clean incremental mode=debug library build under GCC 16 after each fix.

Dependency on #723

The first two commits on this branch are cherry-picked from #723 (template-id-cdtor fix for numeric/MathVector.hh, MathMatrix.hh, and OneDHistogram.hh) — GCC 16 hits the identical issue, and it's needed here just to get past numeric/ far enough to reach the errors this PR actually fixes. No new work is claimed for that part; #723 is the canonical fix and should merge independently of this one. If #723 merges first, rebasing this branch will make those two commits disappear from the diff.

lyskov-ai added 3 commits July 1, 2026 00:39
GCC 15 enforces -Werror=template-id-cdtor: a class template may not name
its own constructors or destructors with explicit template arguments
(the injected-class-name must be used without <...>). MathVector and
MathMatrix declared several ctors and their dtor as e.g.
'MathVector< T>()' and '~MathVector< T>()', which fails to compile under
GCC 15 in C++20 mode and broke the debug build before it reached any
core/ code.

Drop the '< T>' so these match the injected-class-name form already used
by the copy constructors in the same classes. No semantic change.
Same -Werror=template-id-cdtor issue as MathVector/MathMatrix: the
default constructor named itself 'OneDHistogram<key1>()' instead of
using the injected-class-name. This one only surfaces when something
instantiates the ctor (a unit test does), so it slipped past a
library-only build and is what broke CI on the previous version of
this fix.
…e cleanup

GCC 16 (experimental trunk) enables -Werror=unused-but-set-variable more
aggressively and flags a genuine correctness issue in
CacheableResidueTypeSets's copy constructor.

- core/chemical/CacheableResidueTypeSets.cc: the copy constructor passed
  *this (the not-yet-constructed destination object) to the base class
  CacheableData's constructor instead of other (the fully-constructed
  source). Harmless in practice today since CacheableData has no data
  members of its own, but reads from an object mid-construction and is
  the pattern GCC 16 correctly flags as -Wmaybe-uninitialized.

- protocols/simple_moves/MissingDensityToJumpMover.cc: the default
  constructor called MissingDensityToJumpMover::get_name() (a qualified,
  non-virtual call, but still a call through *this) from its own
  mem-initializer-list to build the Mover base class's name argument.
  get_name() just returns a string literal, so this is passed directly
  instead, sidestepping the call-through-a-mid-construction-object
  pattern GCC 16 flags.

- core/scoring/EnergyGraph.hh, core/pose/PDBInfo.cc,
  core/io/mmtf/mmtf_writer.cc, protocols/cartesian/md.cc,
  protocols/denovo_design/components/StructureDataFactory.cc,
  protocols/denovo_design/movers/FoldArchitectMover.cc,
  protocols/forge/methods/pose_mod.hh,
  protocols/noesy_assign/DistanceScoreMover.cc,
  protocols/noesy_assign/StructureDependentPeakCalibrator.cc: remove
  loop counters (iilag, idx, chainIndex, modelIndex, imap, cur_chain,
  count, current_pos, ct_peaks, pose_ct) that are incremented alongside
  a real iterator but never read anywhere in their function. No
  behavior change.

- core/pack/guidance_scoreterms/sap/SapConstraintHelper.cc: offset is
  deliberately tracked "for symmetry" per an existing comment even
  though never read; add an explicit (void) cast rather than removing
  it, preserving that intent while satisfying the warning.
This branch carries the same numeric/ template-id ctor/dtor fix as RosettaCommons#723;
run tools/python_cc_reader/beautify_changed_files_in_branch.py over the
changed files to restore project-standard formatting the template-id edits
had left off: member-initializer lists in MathVector/MathMatrix re-indented
one level under their constructors, plus incidental namespace/data-member
normalization in OneDHistogram.hh so the beautify check passes.

Whitespace-only; no semantic change.
@lyskov
lyskov merged commit 188eabb into RosettaCommons:main Jul 9, 2026
1 check passed
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