Remove legacy threading dead code by zzcgumn · Pull Request #44 · zzcgumn/dds3

zzcgumn · 2026-02-24T08:32:38Z

Summary

Primary objective: remove legacy threading dead code and document the modern API.

This PR:

Removes obsolete threading configuration from the test harness and docs
Adds deprecation guidance for legacy C initialization APIs
Adds migration documentation and example code
Cleans up a broken perf_sample CI job
Fixes DD table calc consistency by reusing a single SolverContext across declarers

Rationale

The dead-code cleanup and documentation were the main goals of this PR. During verification, an unrelated calc consistency bug surfaced, so a targeted fix is included here to keep verification accurate.

Verification

All 28 unit tests pass
dtest -f hands/list100.txt -s calc -n 1 shows no Difference output
dtest -f hands/list100.txt -s solve -n 4 passes

Files Changed

library/src/calc_tables.cpp
library/src/solver_if.cpp
library/src/solver_if.hpp
library/src/api/dll.h
library/tests/* (threading option removal)
docs/* and examples/* (migration guidance)
.github/workflows/ci_linux.yml
copilot/* (task tracking)

- Created comprehensive plan in copilot/plans/remove_dead_code.md - Broke down plan into 5 actionable tasks in copilot/tasks/remove_dead_code/ - Added remove_dead_code.md instructions - Moved completed conform_project_to_coding_standards.md to completed/ Plan addresses removal of obsolete Threading enum, deprecation of legacy init APIs, cleanup of broken CI steps, and documentation of C API migration.

- Removed entire perf_sample job from .github/workflows/ci_linux.yml - Job referenced non-existent copilot/perf/run_sample.sh script - Only ran on manual workflow_dispatch and never functioned correctly - No copilot/perf/ directory exists in repository Task: 01_remove_perf_sample_ci_step.md (Phase 1 - Low Risk)

@deprecated

Mark legacy global initialization functions as deprecated via @deprecated documentation comments, steering users toward the modern SolverContext API. Changes: - library/src/api/dll.h: Add @deprecated notices to SetMaxThreads, SetThreading, SetResources, FreeMemory with migration guidance - docs/api_migration.md: Create comprehensive migration guide with 3 detailed examples (basic solving, batch processing, multi-threading) - README.md: Add API documentation section with side-by-side comparison of modern C++ vs legacy C API - Task file: Update status to completed Approach: Used @deprecated in Doxygen comments instead of [[deprecated]] attributes due to C API compatibility requirements. C functions with extern "C" linkage cannot reliably use C++11 attributes. All tests pass (28/28). No breaking changes - fully backward compatible. Task: 02_add_deprecation_notices.md Plan: copilot/plans/remove_dead_code.md

Remove legacy threading configuration from test infrastructure now that threading is implicit via SolverContext. This cleans up command-line options and reduces unused configuration paths. Changes: - library/tests/cst.hpp: Remove Threading enum and threading_ field - library/tests/args.cpp: Drop -t/--threading option, threading parsing, usage text, and print output; update option count - library/tests/args.hpp: Remove threading mention from docs - library/tests/itest.cpp and dtest.cpp: Remove SetThreading() call - Task file: Mark Task 03 as completed Validation: - bazel build //library/tests:all --compilation_mode=dbg - bazel test //library/tests:all --compilation_mode=dbg - bazel test //... --compilation_mode=dbg - dtest --help shows no threading option - grep confirms no Threading references Task: 03_remove_threading_enum.md Plan: copilot/plans/remove_dead_code.md

Create legacy C API documentation and add a migration example to clarify how the modern SolverContext API replaces global initialization. Update build system docs with API layering and refresh examples to use the current SolverConfig memory fields. Changes: - docs/legacy_c_api.md: New compatibility and migration document - docs/BUILD_SYSTEM.md: Add API layers section - examples/migration_example.cpp: Side-by-side legacy/modern usage - examples/BUILD.bazel: Add migration_example target - README.md: Link migration example - docs/api_migration.md: Fix SolverConfig field names - Task 04 file: Mark complete Validation: - bazel build //examples:migration_example --compilation_mode=dbg Task: 04_document_c_api_compatibility.md Plan: copilot/plans/remove_dead_code.md

Record Task 05 build, test, smoke test, and example run results. Note missing regression BUILD package and unavailable ASAN/clang-tidy configs on this setup, along with observed calc_dd_table output. Task: 05_final_verification_and_pr.md Plan: copilot/plans/remove_dead_code.md

The calc_single_common function was creating separate SolverContext instances for each declarer when calculating DD tables, causing each declarer to have its own isolated transposition table. This led to inconsistent results between declarers solving the same board. Changes: - Modified calc_single_common to create a single SolverContext and pass it to all solver calls - Updated solve_same_board signature to accept SolverContext& instead of ThreadData pointer, ensuring context reuse - All four declarers now share the same transposition table state, eliminating the inconsistency Verification: - All 28 unit tests pass - No "Difference" output when running dtest with calc mode on 100 hands - Solve mode continues to work correctly

Copilot

Pull request overview

This PR combines two distinct sets of changes: (1) a bug fix for DD table calculation consistency by reusing SolverContext across declarers, and (2) removal of legacy threading infrastructure and addition of extensive API migration documentation.

Changes:

Core bug fix: Modified calc_single_common() to create a single SolverContext and reuse it across all four declarers, fixing inconsistent DD table results
Test infrastructure cleanup: Removed obsolete Threading enum and -t/--threading command-line options from test harness
API deprecation: Added deprecation notices to legacy C API initialization functions (SetThreading, SetMaxThreads, SetResources, FreeMemory)
Documentation: Created comprehensive migration guides (docs/api_migration.md, docs/legacy_c_api.md) with examples showing how to migrate from legacy C API to modern C++ API
CI cleanup: Removed non-functional perf_sample job from Linux CI workflow
Examples: Added migration_example.cpp demonstrating both legacy and modern API usage patterns

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
library/src/calc_tables.cpp	Creates single SolverContext and passes to both SolveBoard and solve_same_board for consistency
library/src/solver_if.hpp	Changes solve_same_board signature from ThreadData pointer to SolverContext reference
library/src/solver_if.cpp	Refactored to accept and reuse provided SolverContext instead of creating new one
library/tests/cst.hpp	Removed Threading enum definition and threading_ field from OptionsType
library/tests/args.cpp	Removed threading option parsing logic and updated help text
library/tests/args.hpp	Updated documentation to remove threading model references
library/tests/dtest.cpp	Removed SetThreading() initialization call
library/tests/itest.cpp	Removed SetThreading() initialization call
library/src/api/dll.h	Added deprecation notices to legacy initialization functions
examples/migration_example.cpp	New example showing side-by-side legacy and modern API usage
examples/BUILD.bazel	Added migration_example build target
docs/api_migration.md	New comprehensive migration guide with detailed examples
docs/legacy_c_api.md	New documentation of C API compatibility strategy
docs/BUILD_SYSTEM.md	Added API layers section
README.md	Added API documentation overview with quick examples
.github/workflows/ci_linux.yml	Removed broken perf_sample CI job
copilot/*	Planning and task tracking documentation for the dead code removal work

library/src/calc_tables.cpp

library/src/api/dll.h

copilot/tasks/remove_dead_code/05_final_verification_and_pr.md

library/src/solver_if.cpp

copilot/tasks/remove_dead_code/02_add_deprecation_notices.md

copilot/plans/remove_dead_code.md

copilot/tasks/remove_dead_code/04_document_c_api_compatibility.md

- Fix SolverConfig field name references in docs and deprecation notice - Correct indentation in solve_same_board - Update verification notes for calc smoke test

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.

docs/legacy_c_api.md

examples/migration_example.cpp

docs/api_migration.md

README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ed folders.

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

examples/migration_example.cpp:46

This new example doesn’t match the conventions used by the other files in examples/: they consistently use trailing return types for main (e.g., examples/solve_board.cpp:21 uses auto main() -> int) and a different indentation style. Aligning migration_example.cpp with the existing examples will keep the examples directory consistent.

{
    Deal deal{};
    solve_legacy(deal);
    solve_modern(deal);
    return 0;
}

library/src/api/dll.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 5 comments.

docs/legacy_c_api.md

README.md

examples/migration_example.cpp

library/src/calc_tables.cpp

docs/api_migration.md

- Use simpler SolverContext(cfg) constructor throughout all examples - Improve SetMaxThreads deprecation notice to clarify threading model - Update comment in calc_tables.cpp to mention TransTable reuse - Fix include order in migration_example.cpp - Simplify README.md example to be more self-contained - Apply consistent constructor pattern across all documentation All tests pass (28/28)

- Use auto main() -> int in migration_example.cpp for consistency - Update field names in completed plan doc to match actual SolverConfig

…tices.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…patibility.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.

library/src/calc_tables.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

library/tests/args.cpp

library/src/calc_tables.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 6 comments.

copilot/instructions/completed/remove_dead_code.md

Copilot · 2026-02-26T15:24:07Z

library/src/calc_tables.cpp

  for (int k = 1; k < DDS_HANDS; k++)
  {
    int hint = (k == 2 ? fut.score[0] : 13 - fut.score[0]);

    deal.first = k; // Next declarer


The per-declarer loop computes hint from fut.score[0] and calls solve_same_board() even if the initial SolveBoard() call failed. Since FutureTricks fut is not value-initialized, this can read garbage and lead to undefined behavior. Consider returning early / skipping the loop when res != 1, and/or value-initialize fut before first use.

library/src/calc_tables.cpp

docs/legacy_c_api.md

docs/api_migration.md

README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

- Initialize FutureTricks fut with {} to avoid undefined behavior if first solve fails - Extend context reuse comment to explicitly mention consistency bug fix - Ensures proper zero-initialization of fut structure

Copilot

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

zzcgumn added 7 commits February 23, 2026 20:59

Copilot AI review requested due to automatic review settings February 24, 2026 08:32

Copilot started reviewing on behalf of zzcgumn February 24, 2026 08:33 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

zzcgumn changed the title ~~Fix calc DD table consistency by reusing SolverContext~~ Remove legacy threading dead code Feb 24, 2026

Address PR feedback on naming and docs

382dd48

- Fix SolverConfig field name references in docs and deprecation notice - Correct indentation in solve_same_board - Update verification notes for calc smoke test

zzcgumn requested a review from Copilot February 24, 2026 09:24

Copilot started reviewing on behalf of zzcgumn February 24, 2026 09:24 View session

zzcgumn self-assigned this Feb 24, 2026

Copilot AI reviewed Feb 24, 2026

View reviewed changes

docs/legacy_c_api.md Outdated Show resolved Hide resolved

examples/migration_example.cpp Outdated Show resolved Hide resolved

examples/migration_example.cpp Outdated Show resolved Hide resolved

docs/api_migration.md Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

Update docs/legacy_c_api.md

9a58102

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 24, 2026 09:35

Copilot started reviewing on behalf of zzcgumn February 24, 2026 09:35 View session

zzcgumn and others added 4 commits February 24, 2026 10:36

Update examples/migration_example.cpp

048921b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update examples/migration_example.cpp

9844676

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

f5939ec

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

moves remove dead code instruction, plan and tasks into their complet…

49da607

…ed folders.

Copilot AI reviewed Feb 24, 2026

View reviewed changes

library/src/api/dll.h Outdated Show resolved Hide resolved

Update library/src/api/dll.h

aa26e59

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 25, 2026 20:58

Copilot started reviewing on behalf of zzcgumn February 25, 2026 20:58 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

docs/legacy_c_api.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

examples/migration_example.cpp Show resolved Hide resolved

library/src/calc_tables.cpp Outdated Show resolved Hide resolved

docs/api_migration.md Outdated Show resolved Hide resolved

zzcgumn requested a review from Copilot February 25, 2026 22:06

zzcgumn and others added 2 commits February 25, 2026 22:27

Fix remaining review comments

5f575f8

- Use auto main() -> int in migration_example.cpp for consistency - Update field names in completed plan doc to match actual SolverConfig

Update copilot/tasks/completed/remove_dead_code/02_add_deprecation_no…

d2c1875

…tices.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 25, 2026 22:30

Update copilot/tasks/completed/remove_dead_code/04_document_c_api_com…

4ef2333

…patibility.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started reviewing on behalf of zzcgumn February 25, 2026 22:31 View session

Update copilot/plans/completed/remove_dead_code.md

985e17e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Feb 25, 2026

View reviewed changes

library/src/calc_tables.cpp Show resolved Hide resolved

Update library/src/calc_tables.cpp

0214e75

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 26, 2026 10:08

Copilot started reviewing on behalf of zzcgumn February 26, 2026 10:10 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

library/tests/args.cpp Outdated Show resolved Hide resolved

library/src/calc_tables.cpp Outdated Show resolved Hide resolved

library/src/calc_tables.cpp Outdated Show resolved Hide resolved

Update library/src/calc_tables.cpp

dd91f31

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 26, 2026 15:15

Copilot started reviewing on behalf of zzcgumn February 26, 2026 15:16 View session

Update library/tests/args.cpp

d7ea501

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Update README.md

e5fa724

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 26, 2026 16:59

Update copilot/instructions/completed/remove_dead_code.md

1423248

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started reviewing on behalf of zzcgumn February 26, 2026 17:00 View session

zzcgumn and others added 3 commits February 26, 2026 17:01

Update library/src/calc_tables.cpp

425548e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update docs/legacy_c_api.md

f7565d2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update docs/api_migration.md

085673e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Fix calc_tables.cpp: initialize fut and enhance reuse comment

70bf701

- Initialize FutureTricks fut with {} to avoid undefined behavior if first solve fails - Extend context reuse comment to explicitly mention consistency bug fix - Ensures proper zero-initialization of fut structure

zzcgumn requested a review from Copilot February 27, 2026 18:17

Copilot started reviewing on behalf of zzcgumn February 27, 2026 18:17 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

zzcgumn merged commit 3c8a678 into main Feb 27, 2026
6 checks passed

zzcgumn deleted the chore/remove_dead_code branch February 27, 2026 18:31

Conversation

zzcgumn commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Rationale

Verification

Files Changed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

zzcgumn commented Feb 24, 2026 •

edited

Loading