Deprecate legacy C API functions with dds_c_api.h equivalents - #354
Open
zzcgumn wants to merge 3 commits into
Open
Deprecate legacy C API functions with dds_c_api.h equivalents#354zzcgumn wants to merge 3 commits into
zzcgumn wants to merge 3 commits into
Conversation
…recated Adds @deprecated doxygen tags (matching the existing SetMaxThreads / SetThreading / SetResources / FreeMemory pattern) to the legacy C API functions that have a direct replacement in the modern SolverContext-based API and its dds_c_api.h C-ABI shim: - SolveBoard -> SolveBoard(SolverContext&, ...) / dds_c_solve_board - CalcDDtable, CalcDDtableN -> calc_dd_table(SolverContext&, ...) / dds_c_calc_dd_table - CalcDDtablePBN, CalcDDtablePBNN -> calc_dd_table_pbn(SolverContext&, ...) / dds_c_calc_dd_table_pbn - CalcPar -> calc_par(SolverContext&, ...) / dds_c_calc_par CalcPar previously had no doc comment at all; it now gets a full block. Functions with no dds_c_api.h equivalent (SolveBoardPBN, CalcParPBN, the batch CalcAllTables*/SolveAllBoards*/SolveAllChunks* family, sides/dealer par helpers, text-format converters, play analysis) are left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FNjHJrNuYyeqgMKY4v2urk
…tions Extends the pure-C ABI shim (dds_c_api.h/.cpp) so FFI consumers (Java FFM, .NET, ctypes) have a consistent, complete header to bind against, without needing dll.h for these functions too: Context-free utilities (no SolverContext; forward straight to the existing dll.h PascalCase function since it's already POD-only/extern "C"): - dds_c_par_from_table (mirrors the modern calc_par_from_table name) - dds_c_sides_par, dds_c_dealer_par, dds_c_dealer_par_bin, dds_c_sides_par_bin - dds_c_convert_to_dealer_text_format, dds_c_convert_to_sides_text_format - dds_c_get_dds_info, dds_c_error_message PBN single-board functions (needed new SolverContext-taking C++ overloads first, following the existing calc_dd_table_pbn pattern - convert PBN to binary via the internal convert_from_pbn() and delegate): - solve_board_pbn(SolverContext&, ...) in solve_board.hpp/solver_context_adapter.cpp - calc_par_pbn(SolverContext&, ...) in calc_par.hpp/calc_par.cpp - dds_solve_board_pbn / dds_calc_par_pbn in dds_api.hpp/dds_api.cpp - dds_c_solve_board_pbn / dds_c_calc_par_pbn in dds_c_api.h/.cpp Also marks SolveBoardPBN and CalcParPBN as @deprecated in dll.h now that they have dds_c_api.h equivalents, matching the pattern already applied to SolveBoard/CalcDDtable/CalcPar. CalcParPBN previously had no doc comment; it now gets a full block. Out of scope (per plan): AnalysePlayBin/AnalysePlayPBN (no modern play-analysis C++ API exists yet) and the CalcAllTables*/SolveAllBoards* batch family (would need new heap-based shim entries built on the internal parallel_all_boards_n primitive). SolveAllChunks* are already marked as dying aliases in the codebase and are not touched. Adds null-handle/null-pointer safety tests and functional tests for all eleven new entries in dds_c_api_test.cpp. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FNjHJrNuYyeqgMKY4v2urk
Two independent breakages surfaced by CI on the previous commit: 1. jni/tests:export_set_test failed on every build_and_test job: the two .lds export lists (jni/version_script.lds, jni/exported_symbols.lds) are checked-in, manually-regenerated files, and I forgot to regenerate them after adding the eleven new dds_c_* symbols to dds_c_api.h. Regenerated both via `python3 jni/gen_export_lists.py --headers ... --linux ... --macos ...` per the instructions in jni/BUILD.bazel. 2. DdsCApiParUtilities.ConvertToSidesTextFormatProducesText failed under ASan/MSan with a stack-buffer-overflow/uninitialized-read: the test fed ConvertToSidesTextFormat a single dds_c_dealer_par_bin() result, but the function indexes its input as a 2-element array (pres[0]/pres[1], one entry per side) and expects dds_c_sides_par_bin()'s output instead - a single ParResultsMaster is one element short, so reading the second one ran past the end of the stack variable. Fixed the test to use dds_c_sides_par_bin(), and tightened dds_c_convert_to_sides_text_format's signature/doc comments in dds_c_api.h/.cpp to say `par[2]` and spell out which of the two dealer/sides functions feeds which converter, so the same mistake isn't easy to repeat at a call site. Verified locally: bazel test //library/... //jni/... (62/62 pass) and bazel test --config=asan //library/tests:dds_c_api_test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FNjHJrNuYyeqgMKY4v2urk
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances the DDS 3 API migration by documenting deprecations in the legacy dll.h C API and expanding the stable pure-C shim (dds_c_api.*) so FFI consumers can bind a single header for both context-based solving and context-free utilities.
Changes:
- Adds PBN single-board solve/par entrypoints across the modern C++ layer, exported C++ API, and the pure-C ABI shim.
- Adds context-free par/text/info/error utilities to
dds_c_api.*(noSolverContexthandle required). - Extends unit tests and JNI linker symbol lists to cover/export the new pure-C ABI functions.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| library/tests/dds_c_api_test.cpp | Adds coverage for new PBN solve/par shim functions and the new context-free utility shim functions. |
| library/src/solver_context_adapter.cpp | Implements solve_board_pbn(SolverContext&, ...) by converting PBN to binary and delegating to solve_board. |
| library/src/dds_api.cpp | Exports dds_solve_board_pbn and dds_calc_par_pbn for the modern C++/FFI-facing API layer. |
| library/src/calc_par.cpp | Implements calc_par_pbn(SolverContext&, ...) by converting PBN cards then delegating to calc_par. |
| library/src/api/solve_board.hpp | Declares the new solve_board_pbn(SolverContext&, ...) overload with Doxygen docs. |
| library/src/api/dll.h | Adds Doxygen @deprecated guidance for legacy entrypoints where modern equivalents exist. |
| library/src/api/dds_c_api.h | Adds new pure-C ABI declarations for PBN solve/par and context-free utilities. |
| library/src/api/dds_c_api.cpp | Implements the new pure-C ABI functions with null-guarding and catch-all exception boundaries. |
| library/src/api/dds_api.hpp | Declares the new exported API entrypoints used by the shim (dds_*_pbn, dds_calc_par_pbn). |
| library/src/api/calc_par.hpp | Declares calc_par_pbn(SolverContext&, ...) with documentation. |
| jni/version_script.lds | Exports the additional dds_c_* symbols for JNI builds (ELF version script). |
| jni/exported_symbols.lds | Exports the additional dds_c_* symbols for JNI builds (Mach-O exported symbols list). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+482
to
+484
| struct DDSInfo info = {}; | ||
| dds_c_get_dds_info(&info); | ||
| EXPECT_STREQ(info.version_string, "3.1.0"); |
Comment on lines
+532
to
535
| * @deprecated Use SolverContext with the SolveBoard(SolverContext&, ...) | ||
| * overload instead. | ||
| * See docs/api_migration.md for modern C++ API examples. | ||
| * |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
dll.hfunctions as@deprecatedwherever adds_c_api.h(or modernSolverContext-based C++) equivalent exists:SolveBoard,CalcDDtable[N],CalcDDtablePBN[N],CalcPar,SolveBoardPBN,CalcParPBN.dds_c_api.h/.cpp) with entries for the remaining single-board/utility functions, so FFI consumers (Java FFM, .NET, ctypes) have one consistent header to bind against:dds_c_par_from_table,dds_c_sides_par,dds_c_dealer_par,dds_c_dealer_par_bin,dds_c_sides_par_bin,dds_c_convert_to_dealer_text_format,dds_c_convert_to_sides_text_format,dds_c_get_dds_info,dds_c_error_message.dds_c_solve_board_pbn,dds_c_calc_par_pbn, backed by newsolve_board_pbn/calc_par_pbnSolverContext&overloads in the modern C++ layer.AnalysePlayBin/AnalysePlayPBN(no modern play-analysis C++ API exists yet) and theCalcAllTables*/SolveAllBoards*batch family (would need new heap-based shim entries built on the internalparallel_all_boards_nprimitive).SolveAllChunks*are already marked as dying aliases in the codebase and are untouched.Test plan
bazel build //library/...— succeedsbazel test //library/...— 57/57 test targets passbazel test //library/tests:dds_c_api_test— 25/25 assertions pass, including new null-handle/null-pointer safety and functional tests for all eleven newdds_c_api.hentries🤖 Generated with Claude Code
Closes #294