From 3cbd3ea4af6cdd5cb5aa548f8e918f4a9561d8aa Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 16:28:17 +0100 Subject: [PATCH 01/35] Extend the pure-C shim with the nine missing entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .NET binding calls fourteen modern context functions, but the shim covered only five, so the other nine had no C-ABI equivalent and the Unix shared library could not satisfy them. Adds dds_c_create_solvercontext, dds_c_calc_dd_table_pbn, the TT configure/resize/clear trio, both resets, and the two logging passthroughs. Each follows the file's existing pattern: null guards before use and a catch-all so no C++ exception unwinds through the C ABI boundary. SolverConfig is decomposed into three int parameters rather than mirrored as a C struct — passing a struct by value is the ABI question this shim exists to avoid, and a mirror type would be a second definition to keep in sync. TTKind crosses as an int; the C++ enum class and the C# enum already agree on 0 = Small, 1 = Large. Co-Authored-By: Claude Opus 4.8 --- library/src/api/dds_c_api.cpp | 115 ++++++++++++++++++++++++++++++++++ library/src/api/dds_c_api.h | 27 ++++++++ 2 files changed, 142 insertions(+) diff --git a/library/src/api/dds_c_api.cpp b/library/src/api/dds_c_api.cpp index a66499e6..ff9892e6 100644 --- a/library/src/api/dds_c_api.cpp +++ b/library/src/api/dds_c_api.cpp @@ -92,4 +92,119 @@ DLLEXPORT int dds_c_calc_par(DDS_C_SOLVER_CTX ctx, } } +DLLEXPORT DDS_C_SOLVER_CTX dds_c_create_solvercontext(int tt_kind, + int def_mb, int max_mb) +{ + try { + SolverConfig cfg; + cfg.tt_kind_ = static_cast(tt_kind); + cfg.tt_mem_default_mb_ = def_mb; + cfg.tt_mem_maximum_mb_ = max_mb; + return static_cast(dds_create_solvercontext(cfg)); + } catch (...) { + return nullptr; + } +} + +DLLEXPORT int dds_c_calc_dd_table_pbn(DDS_C_SOLVER_CTX ctx, + const struct DdTableDealPBN* deal, + struct DdTableResults* results) +{ + if (ctx == nullptr || deal == nullptr || results == nullptr) + return RETURN_UNKNOWN_FAULT; + + try { + return dds_calc_dd_table_pbn(static_cast(ctx), + *deal, results); + } catch (...) { + return RETURN_UNKNOWN_FAULT; + } +} + +DLLEXPORT void dds_c_configure_tt(DDS_C_SOLVER_CTX ctx, int tt_kind, + int def_mb, int max_mb) +{ + if (ctx == nullptr) + return; + + try { + dds_configure_tt(static_cast(ctx), + static_cast(tt_kind), def_mb, max_mb); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_resize_tt(DDS_C_SOLVER_CTX ctx, int def_mb, int max_mb) +{ + if (ctx == nullptr) + return; + + try { + dds_resize_tt(static_cast(ctx), def_mb, max_mb); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_clear_tt(DDS_C_SOLVER_CTX ctx) +{ + if (ctx == nullptr) + return; + + try { + dds_clear_tt(static_cast(ctx)); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_reset_for_solve(DDS_C_SOLVER_CTX ctx) +{ + if (ctx == nullptr) + return; + + try { + dds_reset_for_solve(static_cast(ctx)); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_reset_best_moves_lite(DDS_C_SOLVER_CTX ctx) +{ + if (ctx == nullptr) + return; + + try { + dds_reset_best_moves_lite(static_cast(ctx)); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_log_append(DDS_C_SOLVER_CTX ctx, const char* msg) +{ + if (ctx == nullptr || msg == nullptr) + return; + + try { + dds_log_append(static_cast(ctx), msg); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + +DLLEXPORT void dds_c_log_clear(DDS_C_SOLVER_CTX ctx) +{ + if (ctx == nullptr) + return; + + try { + dds_log_clear(static_cast(ctx)); + } catch (...) { + // Must not unwind through the C ABI boundary. + } +} + } // extern "C" diff --git a/library/src/api/dds_c_api.h b/library/src/api/dds_c_api.h index 975b1024..1e11053c 100644 --- a/library/src/api/dds_c_api.h +++ b/library/src/api/dds_c_api.h @@ -54,6 +54,33 @@ DLLEXPORT int dds_c_calc_par(DDS_C_SOLVER_CTX ctx, struct DdTableResults* results, struct ParResults* par); +/* Creation with explicit transposition-table configuration. The C++ SolverConfig + is decomposed into scalars rather than mirrored as a struct: passing a struct + by value is exactly the ABI question this shim exists to avoid, and a mirror + type would be a second definition to keep in sync. tt_kind: 0 = Small, + 1 = Large (matching enum class TTKind). Returns NULL on failure. */ +DLLEXPORT DDS_C_SOLVER_CTX dds_c_create_solvercontext(int tt_kind, + int def_mb, int max_mb); + +/* Compute the double dummy table from a PBN-format deal. */ +DLLEXPORT int dds_c_calc_dd_table_pbn(DDS_C_SOLVER_CTX ctx, + const struct DdTableDealPBN* deal, + struct DdTableResults* results); + +/* Transposition-table configuration. */ +DLLEXPORT void dds_c_configure_tt(DDS_C_SOLVER_CTX ctx, int tt_kind, + int def_mb, int max_mb); +DLLEXPORT void dds_c_resize_tt(DDS_C_SOLVER_CTX ctx, int def_mb, int max_mb); +DLLEXPORT void dds_c_clear_tt(DDS_C_SOLVER_CTX ctx); + +/* Per-solve state resets. */ +DLLEXPORT void dds_c_reset_for_solve(DDS_C_SOLVER_CTX ctx); +DLLEXPORT void dds_c_reset_best_moves_lite(DDS_C_SOLVER_CTX ctx); + +/* Logging passthrough. msg is a NUL-terminated UTF-8 string. */ +DLLEXPORT void dds_c_log_append(DDS_C_SOLVER_CTX ctx, const char* msg); +DLLEXPORT void dds_c_log_clear(DDS_C_SOLVER_CTX ctx); + #ifdef __cplusplus } #endif From 8b1d825cdee9ade16a11ea46c5ee64b3c481c6d4 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 17:01:10 +0100 Subject: [PATCH 02/35] Regenerate export lists for the widened C shim Publishes the nine shim symbols added in the previous commit through the Unix export lists, so libdds.{so,dylib} exports all fourteen dds_c_* entry points instead of five. Both .lds files are generated artifacts, regenerated with gen_export_lists.py rather than hand-edited. export_set_test derives its expectations from the headers, so it validated the result with no wiring change; the FFM smoke tests confirm the JVM binding is unaffected. Co-Authored-By: Claude Opus 4.8 --- jni/exported_symbols.lds | 9 +++++++++ jni/version_script.lds | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/jni/exported_symbols.lds b/jni/exported_symbols.lds index 66d30184..5802587b 100644 --- a/jni/exported_symbols.lds +++ b/jni/exported_symbols.lds @@ -40,7 +40,16 @@ _SolveAllChunksPBN _SolveBoard _SolveBoardPBN _dds_c_calc_dd_table +_dds_c_calc_dd_table_pbn _dds_c_calc_par +_dds_c_clear_tt +_dds_c_configure_tt +_dds_c_create_solvercontext _dds_c_create_solvercontext_default _dds_c_destroy_solvercontext +_dds_c_log_append +_dds_c_log_clear +_dds_c_reset_best_moves_lite +_dds_c_reset_for_solve +_dds_c_resize_tt _dds_c_solve_board diff --git a/jni/version_script.lds b/jni/version_script.lds index ab0e82b2..91bd80c3 100644 --- a/jni/version_script.lds +++ b/jni/version_script.lds @@ -42,9 +42,18 @@ SolveBoard; SolveBoardPBN; dds_c_calc_dd_table; + dds_c_calc_dd_table_pbn; dds_c_calc_par; + dds_c_clear_tt; + dds_c_configure_tt; + dds_c_create_solvercontext; dds_c_create_solvercontext_default; dds_c_destroy_solvercontext; + dds_c_log_append; + dds_c_log_clear; + dds_c_reset_best_moves_lite; + dds_c_reset_for_solve; + dds_c_resize_tt; dds_c_solve_board; local: *; From 11ee4f150e56b7cd2f1415e2a2e09d7211c66921 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 17:47:30 +0100 Subject: [PATCH 03/35] Add C++ coverage for the pure-C shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shim had no dedicated C++ test — its five original functions were covered only indirectly via the JVM smoke tests. That was tolerable for five thin forwarders, less so for fourteen, and the new group includes the shim's first void-returning and first string-taking entry points whose null guards nothing exercised. Covers null-handle and null-argument rejection, both TT kinds, TT reconfiguration, the resets, logging, and PBN/binary DD-table agreement. The reference board matches DdsSmokeTest.java so the JVM, .NET, and C++ bindings share one fixture. Depends on //library/src:dds explicitly rather than only transitively through dds_c_api: dds.cpp carries the constructor that initializes static solver memory, and without a direct edge the linker drops it. Includes one DISABLED_ test recording a pre-existing library bug found while writing this: on a Small TT, clear_tt() releases the pools and a following reset_for_solve() re-inits over them, dereferencing null in TransTableS::init_tt(). Verified reachable through the C++ API with no dds_c_* call involved, so it predates this shim and affects the existing Windows .NET path. Disabled rather than deleted so the fault stays recorded without breaking the build. Co-Authored-By: Claude Opus 4.8 --- library/tests/BUILD.bazel | 24 +++ library/tests/dds_c_api_test.cpp | 287 +++++++++++++++++++++++++++++++ 2 files changed, 311 insertions(+) create mode 100644 library/tests/dds_c_api_test.cpp diff --git a/library/tests/BUILD.bazel b/library/tests/BUILD.bazel index 2a512817..74d0c171 100644 --- a/library/tests/BUILD.bazel +++ b/library/tests/BUILD.bazel @@ -14,6 +14,7 @@ filegroup( "args_test.cpp", # Uses GoogleTest, compiled separately "report_board_timings_test.cpp", # Uses GoogleTest, compiled separately "parse_par_test.cpp", # Uses GoogleTest, compiled separately + "dds_c_api_test.cpp", # Uses GoogleTest, compiled separately ], ), ) @@ -74,6 +75,29 @@ cc_test( ], ) +# Exercises the pure-C shim through its own ABI, including the null guards and +# catch-all wrappers that exist only at that boundary and would be bypassed by +# calling the reference-taking dds_* functions directly. +# +# Depends on //library/src:dds explicitly, not just transitively through +# dds_c_api: dds.cpp carries the __attribute__((constructor)) that initializes +# static solver memory, and without a direct edge the linker drops that object +# file — leaving TransTableS::init_tt() to dereference null on the first solve +# after a TT-kind switch. The shipped //jni:dds_shared already depends on both. +cc_test( + name = "dds_c_api_test", + srcs = ["dds_c_api_test.cpp"], + size = "small", + copts = DDS_CPPOPTS, + linkopts = DDS_LINKOPTS, + local_defines = DDS_LOCAL_DEFINES, + deps = [ + "//library/src:dds", + "//library/src/api:dds_c_api", + "@googletest//:gtest_main", + ], +) + cc_test( name = "test_timer_test", srcs = [ diff --git a/library/tests/dds_c_api_test.cpp b/library/tests/dds_c_api_test.cpp new file mode 100644 index 00000000..4f23f927 --- /dev/null +++ b/library/tests/dds_c_api_test.cpp @@ -0,0 +1,287 @@ +/* + DDS, a bridge double dummy solver. + + Tests the pure-C ABI shim through its own boundary. + + These call dds_c_* rather than the reference-taking dds_* functions on + purpose: the null guards and the catch-all wrappers exist only in the shim, + so exercising the C++ API directly would bypass exactly the code under test. + + The reference board matches jni/java/org/dds/ffm/DdsSmokeTest.java so the + JVM, .NET, and C++ bindings all agree on one fixture. + + See LICENSE and README. +*/ + +#include + +#include + +#include + +namespace { + +// Full 13-card holding bitmask (ranks 2..A), matching the Java/Python fixtures. +constexpr unsigned int kFullSuit = 0x7FFCU; + +// The reference board: North holds all spades, East all hearts, South all +// diamonds, West all clubs. With spades trump and North to lead, North/South +// take all 13 tricks. +constexpr int kExpectedTricks = 13; + +// res_table[strain][hand], flattened 5 strains x 4 hands. Cross-checked against +// the JVM binding's EXPECTED_DD_TABLE. +constexpr int kExpectedDdTable[DDS_STRAINS][DDS_HANDS] = { + {13, 0, 13, 0}, // spades + {0, 13, 0, 13}, // hearts + {13, 0, 13, 0}, // diamonds + {0, 13, 0, 13}, // clubs + {0, 0, 0, 0}, // no-trump +}; + +// The same board in PBN: ... per hand. +constexpr const char* kReferencePbn = + "N:AKQJT98765432... .AKQJT98765432.. ..AKQJT98765432. ...AKQJT98765432"; + +struct Deal MakeReferenceDeal() +{ + struct Deal dl = {}; // value-initialize; the shim does not zero for us + dl.trump = 0; // spades + dl.first = 0; // North leads + dl.remainCards[0][0] = kFullSuit; // North spades + dl.remainCards[1][1] = kFullSuit; // East hearts + dl.remainCards[2][2] = kFullSuit; // South diamonds + dl.remainCards[3][3] = kFullSuit; // West clubs + return dl; +} + +struct DdTableDeal MakeReferenceTableDeal() +{ + struct DdTableDeal deal = {}; + deal.cards[0][0] = kFullSuit; + deal.cards[1][1] = kFullSuit; + deal.cards[2][2] = kFullSuit; + deal.cards[3][3] = kFullSuit; + return deal; +} + +// Solve the reference board on ctx and return the trick count. +int SolveReference(DDS_C_SOLVER_CTX ctx) +{ + const struct Deal dl = MakeReferenceDeal(); + struct FutureTricks fut = {}; + const int rc = dds_c_solve_board(ctx, &dl, -1, 1, 1, &fut); + EXPECT_EQ(rc, RETURN_NO_FAULT); + return fut.score[0]; +} + +// --------------------------------------------------------------------------- +// Null-handle safety. Every entry point must reject a null handle rather than +// dereferencing it: the int-returning ones with RETURN_UNKNOWN_FAULT, the +// void-returning ones by returning quietly. +// --------------------------------------------------------------------------- + +TEST(DdsCApiNullHandle, IntReturningEntryPointsFailFast) +{ + const struct Deal dl = MakeReferenceDeal(); + const struct DdTableDeal table_deal = MakeReferenceTableDeal(); + struct DdTableDealPBN pbn_deal = {}; + struct FutureTricks fut = {}; + struct DdTableResults results = {}; + struct ParResults par = {}; + + EXPECT_EQ(dds_c_solve_board(nullptr, &dl, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_dd_table(nullptr, &table_deal, &results), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_dd_table_pbn(nullptr, &pbn_deal, &results), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_par(nullptr, &table_deal, 0, &results, &par), RETURN_UNKNOWN_FAULT); +} + +TEST(DdsCApiNullHandle, VoidReturningEntryPointsAreNoOps) +{ + // Each must return without dereferencing; the test passing is the assertion. + dds_c_destroy_solvercontext(nullptr); + dds_c_configure_tt(nullptr, 1, 0, 0); + dds_c_resize_tt(nullptr, 0, 0); + dds_c_clear_tt(nullptr); + dds_c_reset_for_solve(nullptr); + dds_c_reset_best_moves_lite(nullptr); + dds_c_log_append(nullptr, "ignored"); + dds_c_log_clear(nullptr); + SUCCEED(); +} + +TEST(DdsCApiNullArgument, PointerArgumentsAreValidated) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + struct DdTableResults results = {}; + struct DdTableDealPBN pbn_deal = {}; + const struct DdTableDeal table_deal = MakeReferenceTableDeal(); + struct FutureTricks fut = {}; + + EXPECT_EQ(dds_c_solve_board(ctx, nullptr, -1, 1, 1, &fut), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_dd_table_pbn(ctx, nullptr, &results), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_dd_table_pbn(ctx, &pbn_deal, nullptr), RETURN_UNKNOWN_FAULT); + EXPECT_EQ(dds_c_calc_par(ctx, &table_deal, 0, &results, nullptr), RETURN_UNKNOWN_FAULT); + + // A null message must be ignored rather than passed through to strlen. + dds_c_log_append(ctx, nullptr); + + dds_c_destroy_solvercontext(ctx); +} + +// --------------------------------------------------------------------------- +// Functional paths for the newly added entry points. +// --------------------------------------------------------------------------- + +class DdsCApiConfiguredContext : public testing::TestWithParam {}; + +TEST_P(DdsCApiConfiguredContext, SolvesReferenceBoard) +{ + // tt_kind 0 = Small, 1 = Large; both must produce a usable context. + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext(GetParam(), 0, 0); + ASSERT_NE(ctx, nullptr); + + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +INSTANTIATE_TEST_SUITE_P(BothTtKinds, DdsCApiConfiguredContext, + testing::Values(0, 1)); + +TEST(DdsCApiTtConfiguration, ContextRemainsUsableAfterReconfiguration) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + ASSERT_EQ(SolveReference(ctx), kExpectedTricks); + + // Reconfigure, resize, and clear the TT, then confirm the context still + // solves correctly — the point is that these calls do not corrupt state. + dds_c_configure_tt(ctx, 0, 1, 2); + dds_c_resize_tt(ctx, 1, 2); + dds_c_clear_tt(ctx); + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +TEST(DdsCApiResets, ResetsLeaveContextUsable) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + ASSERT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_reset_for_solve(ctx); + dds_c_reset_best_moves_lite(ctx); + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +// Pre-existing library bug, NOT a shim defect — disabled so it documents the +// fault without breaking the build. +// +// Sequence: solve, switch the TT kind to Small, clear_tt(), then +// reset_for_solve(). clear_tt() routes to TransTableS::return_all_memory(), +// which releases the pools; reset_for_solve() then calls +// reset_memory(ResetReason::FreeMemory) -> init_tt(), which dereferences the +// now-null pw_/pl_ pools and segfaults. +// +// Verified reachable through the reference-taking C++ API +// (dds_configure_tt / dds_clear_tt / dds_reset_for_solve) with no dds_c_* +// call involved, so it predates this shim and affects the existing Windows +// .NET path too. The Large TT (the default) is unaffected. +// +// Re-enable once TransTableS reallocates its pools on reset. +TEST(DdsCApiTtConfiguration, DISABLED_SmallTtClearThenResetForSolve) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + ASSERT_EQ(SolveReference(ctx), kExpectedTricks); + dds_c_configure_tt(ctx, 0 /* Small */, 1, 2); + dds_c_clear_tt(ctx); + dds_c_reset_for_solve(ctx); // <-- segfaults today + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +TEST(DdsCApiLogging, AppendAndClearAreCallable) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + dds_c_log_append(ctx, "dds_c_api_test"); + dds_c_log_append(ctx, ""); + dds_c_log_clear(ctx); + + // Logging must not disturb solving. + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + +TEST(DdsCApiDdTable, BinaryTableMatchesExpected) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + const struct DdTableDeal deal = MakeReferenceTableDeal(); + struct DdTableResults results = {}; + ASSERT_EQ(dds_c_calc_dd_table(ctx, &deal, &results), RETURN_NO_FAULT); + + for (int strain = 0; strain < DDS_STRAINS; ++strain) + for (int hand = 0; hand < DDS_HANDS; ++hand) + EXPECT_EQ(results.res_table[strain][hand], kExpectedDdTable[strain][hand]) + << "res_table[" << strain << "][" << hand << "]"; + + dds_c_destroy_solvercontext(ctx); +} + +TEST(DdsCApiDdTable, PbnTableMatchesBinaryTable) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + const struct DdTableDeal binary_deal = MakeReferenceTableDeal(); + struct DdTableResults binary_results = {}; + ASSERT_EQ(dds_c_calc_dd_table(ctx, &binary_deal, &binary_results), + RETURN_NO_FAULT); + + struct DdTableDealPBN pbn_deal = {}; + std::snprintf(pbn_deal.cards, sizeof pbn_deal.cards, "%s", kReferencePbn); + struct DdTableResults pbn_results = {}; + ASSERT_EQ(dds_c_calc_dd_table_pbn(ctx, &pbn_deal, &pbn_results), + RETURN_NO_FAULT); + + for (int strain = 0; strain < DDS_STRAINS; ++strain) + for (int hand = 0; hand < DDS_HANDS; ++hand) + EXPECT_EQ(pbn_results.res_table[strain][hand], + binary_results.res_table[strain][hand]) + << "res_table[" << strain << "][" << hand << "]"; + + dds_c_destroy_solvercontext(ctx); +} + +TEST(DdsCApiPar, ProducesNonEmptyScore) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + const struct DdTableDeal deal = MakeReferenceTableDeal(); + struct DdTableResults results = {}; + struct ParResults par = {}; + ASSERT_EQ(dds_c_calc_par(ctx, &deal, 0 /* vulnerable: none */, &results, &par), + RETURN_NO_FAULT); + + EXPECT_GT(std::strlen(par.par_score[0]), 0U); + + dds_c_destroy_solvercontext(ctx); +} + +} // namespace From 0537e8b797979e73231e5a5d8a13e102b44ceca1 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 18:01:32 +0100 Subject: [PATCH 04/35] Fix null deref in TransTableS::reset_memory after memory release return_all_memory() frees pw_/pn_/pl_ and clears tt_in_use_, leaving make_tt() to reallocate lazily before the next lookup. reset_memory() did not honour that flag: it called init_tt() unconditionally, which reads pw_[0] and segfaults on the freed pools. Reachable from the public API as configure_tt(Small) -> clear_tt() -> reset_for_solve() with no shim involved, so it also affects the existing C++ and Windows .NET paths. The Large TT was never affected because TransTableL::reset_memory() already guards the equivalent case with `pool_ == nullptr`; this adds the direct analogue for the Small TT. Found while adding shim coverage; the previously DISABLED_ regression test in dds_c_api_test is now enabled and passing. Co-Authored-By: Claude Opus 4.8 --- library/src/trans_table/trans_table_s.cpp | 9 +++++++++ library/tests/dds_c_api_test.cpp | 24 +++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/library/src/trans_table/trans_table_s.cpp b/library/src/trans_table/trans_table_s.cpp index 5cacd1e8..eaa57240 100644 --- a/library/src/trans_table/trans_table_s.cpp +++ b/library/src/trans_table/trans_table_s.cpp @@ -343,6 +343,15 @@ auto TransTableS::init_tt() -> void auto TransTableS::reset_memory( [[maybe_unused]] const ResetReason reason) -> void { + // Nothing to reset when the pools have been returned: return_all_memory() + // frees pw_/pn_/pl_ and clears tt_in_use_, and make_tt() reallocates lazily + // before the next lookup. Without this guard init_tt() below dereferences + // the freed pools (pw_[0]) and segfaults — reachable from the public API as + // configure_tt(Small) -> clear_tt() -> reset_for_solve(). TransTableL's + // reset_memory() already guards the equivalent case with `pool_ == nullptr`. + if (!tt_in_use_) + return; + wipe(); init_tt(); diff --git a/library/tests/dds_c_api_test.cpp b/library/tests/dds_c_api_test.cpp index 4f23f927..dc51322f 100644 --- a/library/tests/dds_c_api_test.cpp +++ b/library/tests/dds_c_api_test.cpp @@ -182,22 +182,12 @@ TEST(DdsCApiResets, ResetsLeaveContextUsable) dds_c_destroy_solvercontext(ctx); } -// Pre-existing library bug, NOT a shim defect — disabled so it documents the -// fault without breaking the build. -// -// Sequence: solve, switch the TT kind to Small, clear_tt(), then -// reset_for_solve(). clear_tt() routes to TransTableS::return_all_memory(), -// which releases the pools; reset_for_solve() then calls -// reset_memory(ResetReason::FreeMemory) -> init_tt(), which dereferences the -// now-null pw_/pl_ pools and segfaults. -// -// Verified reachable through the reference-taking C++ API -// (dds_configure_tt / dds_clear_tt / dds_reset_for_solve) with no dds_c_* -// call involved, so it predates this shim and affects the existing Windows -// .NET path too. The Large TT (the default) is unaffected. -// -// Re-enable once TransTableS reallocates its pools on reset. -TEST(DdsCApiTtConfiguration, DISABLED_SmallTtClearThenResetForSolve) +// Regression: on a Small TT, clear_tt() returns the pools and a following +// reset_for_solve() used to re-init over them, dereferencing null in +// TransTableS::init_tt(). The Large TT (the default) was never affected +// because TransTableL::reset_memory() already guarded the equivalent case. +// Reachable from the public API, so this covers the C++ and .NET paths too. +TEST(DdsCApiTtConfiguration, SmallTtClearThenResetForSolve) { DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); ASSERT_NE(ctx, nullptr); @@ -205,7 +195,7 @@ TEST(DdsCApiTtConfiguration, DISABLED_SmallTtClearThenResetForSolve) ASSERT_EQ(SolveReference(ctx), kExpectedTricks); dds_c_configure_tt(ctx, 0 /* Small */, 1, 2); dds_c_clear_tt(ctx); - dds_c_reset_for_solve(ctx); // <-- segfaults today + dds_c_reset_for_solve(ctx); EXPECT_EQ(SolveReference(ctx), kExpectedTricks); dds_c_destroy_solvercontext(ctx); From 8437c7e5021990591e3d8d5eeb905fb9236059ac Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 18:11:44 +0100 Subject: [PATCH 05/35] Retarget the .NET binding onto the pure-C shim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fourteen modern P/Invokes bound dds_* from dds_api.hpp, which the shared library does not export on Linux or macOS — every one of them would have thrown EntryPointNotFoundException there. They now bind the dds_c_* shim via EntryPoint, so the same managed code works on all three platforms. Managed method names are unchanged, so no call site in DDS.cs moves. The one exception is context creation, which takes scalars now that SolverConfig no longer crosses the ABI; SolverContext's constructor unpacks the config it already holds. TTKind needs no change: it is `enum TTKind : int` and marshals as the shim's int parameter. Also renames the library to "dds" (letting .NET's probing supply the lib prefix and per-OS extension), makes dds_log_append's UTF-8 marshalling explicit instead of relying on the platform-dependent CharSet.Ansi default, and deletes the commented-out calc_par and calc_par_from_table declarations — those are plain C++ with no extern "C" or DLLEXPORT, so they were never bindable on any platform. The 30 legacy flat-API P/Invokes are untouched; they already resolve. Note: DDS_Core.slnx still build-depends on solution/dds_native.vcxproj, which now emits a library name nothing loads. Retiring that project is deferred per the plan. Co-Authored-By: Claude Opus 4.8 --- dotnet/DDS_Core/DataModel/SolverContext.cs | 6 +- dotnet/DDS_Core/Native/DdsNative.cs | 68 +++++++++++----------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/dotnet/DDS_Core/DataModel/SolverContext.cs b/dotnet/DDS_Core/DataModel/SolverContext.cs index bdf29cc7..cb288263 100644 --- a/dotnet/DDS_Core/DataModel/SolverContext.cs +++ b/dotnet/DDS_Core/DataModel/SolverContext.cs @@ -17,7 +17,11 @@ public SolverContext() public SolverContext(SolverConfig config) { - Handle = DdsNative.dds_create_solvercontext(config) + // Unpacked into scalars: the native shim is pointer-only and + // POD-only, so SolverConfig never crosses the ABI boundary. + Handle = DdsNative.dds_create_solvercontext( (int) config.TTKind + , config.DefaultMemoryMB + , config.MaximumMemoryMB) ?? throw new InvalidOperationException("Failed to create SolverContext."); } diff --git a/dotnet/DDS_Core/Native/DdsNative.cs b/dotnet/DDS_Core/Native/DdsNative.cs index 8b9b7cf8..24294159 100644 --- a/dotnet/DDS_Core/Native/DdsNative.cs +++ b/dotnet/DDS_Core/Native/DdsNative.cs @@ -5,49 +5,67 @@ namespace DDS_Core.Native; internal static class DdsNative { - private const string DllName = "dds_native"; + // One native library for every platform. .NET's probing supplies the "lib" + // prefix and the per-OS extension, so this single name resolves + // libdds.dylib, libdds.so, and dds.dll. + private const string DllName = "dds"; + + // The modern context entry points below bind the pure-C shim (dds_c_*) + // rather than the reference-taking dds_* functions in dds_api.hpp. Only the + // shim is exported by the shared library on Linux and macOS; the managed + // method names are kept as-is so call sites are unaffected. #region ====== Version 3 specific methods ====== #region ===== Solver Context Management ====== - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_create_solvercontext_default", CallingConvention = CallingConvention.Cdecl)] internal static extern SolverContextHandle dds_create_solvercontext_default(); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] - internal static extern SolverContextHandle dds_create_solvercontext(SolverConfig cfg); + // SolverConfig is passed as scalars: the shim is pointer-only and + // POD-only by design, so no struct crosses the ABI boundary. + [DllImport(DllName, EntryPoint = "dds_c_create_solvercontext", CallingConvention = CallingConvention.Cdecl)] + internal static extern SolverContextHandle dds_create_solvercontext( int ttKind + , int defMB + , int maxMB); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_destroy_solvercontext", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_destroy_solvercontext(IntPtr ctx); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + // TTKind is `enum TTKind : int`, which marshals as a plain int and + // matches the shim's int tt_kind parameter, so the managed + // signature is unchanged. + [DllImport(DllName, EntryPoint = "dds_c_configure_tt", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_configure_tt( SolverContextHandle ctx , TTKind kind , int defMB , int maxMB); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_resize_tt", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_resize_tt( SolverContextHandle ctx , int defMB , int maxMB); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_clear_tt", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_clear_tt(SolverContextHandle ctx); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_reset_for_solve", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_reset_for_solve(SolverContextHandle ctx); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_reset_best_moves_lite", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_reset_best_moves_lite(SolverContextHandle ctx); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)] - internal static extern void dds_log_append( SolverContextHandle ctx, string msg); + // The shim documents msg as NUL-terminated UTF-8; be explicit rather + // than relying on the platform-dependent CharSet.Ansi default. + [DllImport(DllName, EntryPoint = "dds_c_log_append", CallingConvention = CallingConvention.Cdecl)] + internal static extern void dds_log_append( SolverContextHandle ctx + , [MarshalAs(UnmanagedType.LPUTF8Str)] string msg); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_log_clear", CallingConvention = CallingConvention.Cdecl)] internal static extern void dds_log_clear( SolverContextHandle ctx); #endregion #region ====== SolverContext methods ====== - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_solve_board", CallingConvention = CallingConvention.Cdecl)] public static extern int dds_solve_board( SolverContextHandle ctx , in Deal dl , int target @@ -56,40 +74,24 @@ public static extern int dds_solve_board( SolverContextHandle ctx , out FutureTricks fut); #region Call_dd - // [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] - //public static extern int dds_calc_dd_table( in DdTableDeal table_deal - // , out DdTableResults table_results); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_calc_dd_table", CallingConvention = CallingConvention.Cdecl)] public static extern int dds_calc_dd_table( SolverContextHandle ctx , in DdTableDeal table_deal , out DdTableResults table_results); - // [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] - //public static extern int dds_calc_dd_table_pbn( in DdTableDealPBN table_deal_pbn - // , out DdTableResults table_results); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_calc_dd_table_pbn", CallingConvention = CallingConvention.Cdecl)] public static extern int dds_calc_dd_table_pbn( SolverContextHandle ctx , in DdTableDealPBN table_deal_pbn , out DdTableResults table_results); #endregion #region Call_par - // [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] - //public static extern int calc_par( in DdTableDeal table_deal - // , int vulnerable - // , out DdTableResults table_results - // , out ParResults par_results); - [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] + [DllImport(DllName, EntryPoint = "dds_c_calc_par", CallingConvention = CallingConvention.Cdecl)] public static extern int dds_calc_par( SolverContextHandle ctx , in DdTableDeal table_deal , int vulnerable , out DdTableResults table_results , out ParResults par_results); - - // [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)] - //public static extern int calc_par_from_table( in DdTableResults table_results - // , int vulnerable - // , out ParResults par_results); #endregion #endregion #endregion From bd4de6b987a2e53083cab45078124086343d6ed5 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 18:56:17 +0100 Subject: [PATCH 06/35] Add explicit native-library resolution via DDS_LIBRARY_PATH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default probing stays in force — that is what a NuGet package laying the library out under runtimes//native will rely on — but setting DDS_LIBRARY_PATH to a full path now overrides it, so tests and development builds can bind against a freshly-built bazel-bin/jni/libdds.dylib without installing anything. This is the .NET counterpart of the JVM binding's -Ddds.library.path. If the variable is set but the library will not load, the failure is raised naming the attempted path rather than falling through to probing: a typo in a test script should not surface later as a missing entry point. Registered from DdsNative's static constructor rather than a [ModuleInitializer]: the runtime guarantees a type initializer runs before that type's first P/Invoke, so it is equally safe while being lazy rather than eager — and CA2255 warns against module initializers in library code. The task sketched the module-initializer form; this is the same guarantee without the warning. Verified end to end on macOS/arm64: solving the reference board through SolverContext against the Bazel-built library returns 13 tricks, an unset variable falls back to probing, and a wrong one fails loudly. Co-Authored-By: Claude Opus 4.8 --- dotnet/DDS_Core/Native/DdsNative.cs | 4 ++ dotnet/DDS_Core/Native/DdsNativeResolver.cs | 62 +++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 dotnet/DDS_Core/Native/DdsNativeResolver.cs diff --git a/dotnet/DDS_Core/Native/DdsNative.cs b/dotnet/DDS_Core/Native/DdsNative.cs index 24294159..d0c260cb 100644 --- a/dotnet/DDS_Core/Native/DdsNative.cs +++ b/dotnet/DDS_Core/Native/DdsNative.cs @@ -10,6 +10,10 @@ internal static class DdsNative // libdds.dylib, libdds.so, and dds.dll. private const string DllName = "dds"; + // Runs before this type's first P/Invoke, so the DDS_LIBRARY_PATH override + // is always in place without consumers having to call anything. + static DdsNative() => DdsNativeResolver.Register(); + // The modern context entry points below bind the pure-C shim (dds_c_*) // rather than the reference-taking dds_* functions in dds_api.hpp. Only the // shim is exported by the shared library on Linux and macOS; the managed diff --git a/dotnet/DDS_Core/Native/DdsNativeResolver.cs b/dotnet/DDS_Core/Native/DdsNativeResolver.cs new file mode 100644 index 00000000..e5f342a8 --- /dev/null +++ b/dotnet/DDS_Core/Native/DdsNativeResolver.cs @@ -0,0 +1,62 @@ +using System.Reflection; +using System.Runtime.InteropServices; + +namespace DDS_Core.Native; + +/// +/// Resolves the native DDS library for this assembly's P/Invokes. +/// +/// +/// +/// By default the runtime's own probing applies, which is what a NuGet package +/// laying the library out under runtimes/<rid>/native relies on. +/// Setting the DDS_LIBRARY_PATH environment variable to the full path of +/// a library file overrides that, which is how tests and development builds bind +/// against a freshly-built bazel-bin/jni/libdds.dylib without installing +/// anything. It is the .NET counterpart of the JVM binding's +/// -Ddds.library.path. +/// +/// +/// If DDS_LIBRARY_PATH is set but the library cannot be loaded, the +/// failure is surfaced with the attempted path rather than silently falling back +/// to probing: a typo in a test script should not present later as a missing +/// entry point. +/// +/// +internal static class DdsNativeResolver +{ + /// Environment variable holding an explicit path to the native library. + internal const string LibraryPathVariable = "DDS_LIBRARY_PATH"; + + /// + /// Registers the resolver. Called from 's static + /// constructor, which the runtime guarantees runs before that type's first + /// P/Invoke — so no explicit setup call is needed from consumers. A module + /// initializer would also work but runs eagerly at load time, which is both + /// more surprising in a library and flagged by CA2255. + /// + internal static void Register() + => NativeLibrary.SetDllImportResolver(typeof(DdsNative).Assembly, Resolve); + + private static IntPtr Resolve(string libraryName, Assembly assembly, DllImportSearchPath? searchPath) + { + // Never intercept imports belonging to anything but the DDS library. + if (!string.Equals(libraryName, "dds", StringComparison.Ordinal)) + return IntPtr.Zero; + + var explicitPath = Environment.GetEnvironmentVariable(LibraryPathVariable); + if (string.IsNullOrWhiteSpace(explicitPath)) + return IntPtr.Zero; // Fall back to the runtime's default probing. + + try + { + return NativeLibrary.Load(explicitPath); + } + catch (Exception ex) + { + throw new DllNotFoundException( + $"{LibraryPathVariable} is set to '{explicitPath}', but the native DDS " + + "library could not be loaded from there.", ex); + } + } +} From e3664d13dff5feb7ee1539683bc96557a6a07306 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 19:01:34 +0100 Subject: [PATCH 07/35] Declare the .NET projects AnyCPU rather than x64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both projects declared x64 while emitting an architecture-neutral assembly, so the declaration did not match what was built. Correcting the task's premise: this pin was NOT blocking Apple Silicon. only declares the valid platform list for the IDE/solution; it never set , so the assembly was already AnyCPU (PE machine 0x014c) and both `dotnet build` and `-p:Platform=AnyCPU` already succeeded on arm64. The change is therefore declarative — it stops the project claiming an architecture it does not target, and unrestricts the IDE configuration list — not an unblocking fix. Output paths verified unchanged: Directory.Build.props interpolates $(platform) into BaseIntermediateOutputPath, but Directory.Build.props is imported before the SDK defines Platform, so that property was already empty and the segment already collapsed. No empty or doubled path segments after the change. Co-Authored-By: Claude Opus 4.8 --- dotnet/DDS_Core/DDS_Core.csproj | 3 ++- dotnet/DDS_Core_Demo/DDS_Core_Demo.csproj | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dotnet/DDS_Core/DDS_Core.csproj b/dotnet/DDS_Core/DDS_Core.csproj index 35151565..72831453 100644 --- a/dotnet/DDS_Core/DDS_Core.csproj +++ b/dotnet/DDS_Core/DDS_Core.csproj @@ -4,7 +4,8 @@ net8.0 enable disable - x64 + + AnyCPU false ..\..\Build\bin\ True diff --git a/dotnet/DDS_Core_Demo/DDS_Core_Demo.csproj b/dotnet/DDS_Core_Demo/DDS_Core_Demo.csproj index ee83a962..ea2df6ef 100644 --- a/dotnet/DDS_Core_Demo/DDS_Core_Demo.csproj +++ b/dotnet/DDS_Core_Demo/DDS_Core_Demo.csproj @@ -5,7 +5,8 @@ net8.0 enable enable - x64 + + AnyCPU false ..\..\Build\bin\ From 469d5c8949b569f42b5eee14361e14ba0f654ba6 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 19:21:19 +0100 Subject: [PATCH 08/35] Add the managed test project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nineteen tests covering the retargeted binding, all passing on macOS/arm64 against the Bazel-built libdds.dylib. The layout tests carry the most weight. The managed structs had only ever been exercised against the MSVC ABI, and a mismatch on SysV or AArch64 corrupts results silently rather than throwing — so the smoke tests alone could not catch it. Expected sizes and offsets are derived from the C headers via offsetof/sizeof, not from running the C#, since asserting what the managed code already does would prove nothing. They pass, which is the first confirmation the layouts are correct off Windows. Between the smoke and lifecycle tests every one of the fourteen retargeted P/Invokes is exercised, so a missing EntryPoint fails here rather than in a consumer. Includes a managed regression for the Small-TT ClearTT/ResetForSolve crash fixed earlier in this branch, and a one-context-per-thread concurrency check. Targets net8.0 to match the library, with Major so the suite also runs where only a newer major runtime is installed (the Homebrew SDK case). This replaces the multi-targeting the task suggested: multi-targeting would have run both frameworks and still needed the roll-forward for the net8.0 pass, so it removed nothing. Where an 8.0 runtime exists, as on CI, the property has no effect. Output paths are inherited from Directory.Build.props rather than overridden: Build/int is gitignored where the SDK-default obj/ is not, and overriding BaseIntermediateOutputPath in the csproj comes too late for MSBuild (MSB3539). Note: DDS_Core.slnx does not build under `dotnet build` because it includes the C++ dds_native.vcxproj, which needs Visual Studio's MSBuild. That predates this change; the projects build individually, which is what CI and `dotnet test` use. Co-Authored-By: Claude Opus 4.8 --- .../DDS_Core.Tests/ContextLifecycleTests.cs | 125 ++++++++++++++++++ dotnet/DDS_Core.Tests/DDS_Core.Tests.csproj | 41 ++++++ dotnet/DDS_Core.Tests/LayoutTests.cs | 83 ++++++++++++ dotnet/DDS_Core.Tests/SmokeTests.cs | 64 +++++++++ dotnet/DDS_Core.Tests/TestDeals.cs | 54 ++++++++ dotnet/DDS_Core/DDS_Core.slnx | 6 + 6 files changed, 373 insertions(+) create mode 100644 dotnet/DDS_Core.Tests/ContextLifecycleTests.cs create mode 100644 dotnet/DDS_Core.Tests/DDS_Core.Tests.csproj create mode 100644 dotnet/DDS_Core.Tests/LayoutTests.cs create mode 100644 dotnet/DDS_Core.Tests/SmokeTests.cs create mode 100644 dotnet/DDS_Core.Tests/TestDeals.cs diff --git a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs new file mode 100644 index 00000000..202a759e --- /dev/null +++ b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs @@ -0,0 +1,125 @@ +using DDS_Core; + +namespace DDS_Core.Tests; + +/// +/// Covers the context-management entry points added to the C shim by this work: +/// config-based construction, TT configuration, the resets, logging, and +/// SafeHandle-driven disposal. Between these and , every +/// one of the fourteen retargeted P/Invokes is exercised — so a missing +/// EntryPoint fails here rather than in a consumer. +/// +public class ContextLifecycleTests +{ + [Theory] + [InlineData(TTKind.Small)] + [InlineData(TTKind.Large)] + public void ConstructedFromConfig_SolvesReferenceDeal(TTKind kind) + { + // Exercises dds_c_create_solvercontext, whose SolverConfig is unpacked + // into scalars at the ABI boundary. + using var ctx = new SolverContext(new SolverConfig(kind, 0, 0)); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + [Fact] + public void TtReconfiguration_LeavesContextUsable() + { + using var ctx = new SolverContext(); + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks _); + + ctx.ConfigureTT(TTKind.Small, 1, 2); + ctx.ResizeTT(1, 2); + ctx.ClearTT(); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + [Fact] + public void Resets_LeaveContextUsable() + { + using var ctx = new SolverContext(); + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks _); + + ctx.ResetForSolve(); + ctx.ResetBestMovesLite(); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + /// + /// Regression for the Small-TT crash fixed alongside this binding work: + /// ClearTT() released the transposition-table pools and a following + /// ResetForSolve() re-initialised over them, faulting inside + /// TransTableS::init_tt(). Reachable from managed code exactly as written + /// here, so this is the .NET-side guard for that fix. + /// + [Fact] + public void SmallTt_ClearThenResetForSolve_DoesNotCrash() + { + using var ctx = new SolverContext(); + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks _); + + ctx.ConfigureTT(TTKind.Small, 1, 2); + ctx.ClearTT(); + ctx.ResetForSolve(); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + [Fact] + public void Logging_DoesNotDisturbSolving() + { + using var ctx = new SolverContext(); + + ctx.LogAppend("DDS_Core.Tests"); + ctx.LogAppend(string.Empty); + ctx.LogClear(); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + /// + /// Disposal must release the native context through SafeHandle without + /// faulting, and must be safe to repeat. + /// + [Fact] + public void Dispose_ReleasesHandleAndIsIdempotent() + { + var ctx = new SolverContext(); + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks _); + + ctx.Dispose(); + ctx.Dispose(); + + Assert.True(ctx.Handle.IsClosed || ctx.Handle.IsInvalid); + } + + /// + /// Contexts are single-threaded, so concurrent use means one context per + /// thread. This is the arrangement the docs prescribe; if it regressed, + /// multi-threaded consumers would corrupt results rather than fail loudly. + /// + [Fact] + public void OneContextPerThread_SolvesConcurrently() + { + const int threads = 4; + var results = new int[threads]; + + Parallel.For(0, threads, i => + { + using var ctx = new SolverContext(); + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + results[i] = fut.Score[0]; + }); + + Assert.All(results, r => Assert.Equal(TestDeals.ExpectedTricks, r)); + } +} diff --git a/dotnet/DDS_Core.Tests/DDS_Core.Tests.csproj b/dotnet/DDS_Core.Tests/DDS_Core.Tests.csproj new file mode 100644 index 00000000..f71123e0 --- /dev/null +++ b/dotnet/DDS_Core.Tests/DDS_Core.Tests.csproj @@ -0,0 +1,41 @@ + + + + + net8.0 + enable + enable + false + AnyCPU + + + Major + + + + + + + + + + + + + + + + + + + + diff --git a/dotnet/DDS_Core.Tests/LayoutTests.cs b/dotnet/DDS_Core.Tests/LayoutTests.cs new file mode 100644 index 00000000..4a66f95f --- /dev/null +++ b/dotnet/DDS_Core.Tests/LayoutTests.cs @@ -0,0 +1,83 @@ +using System.Runtime.InteropServices; +using DDS_Core; + +namespace DDS_Core.Tests; + +/// +/// Pins the managed struct layouts to the C structs in library/src/api/dll.h. +/// +/// +/// +/// These matter more than they look. The managed types were only ever exercised +/// against the MSVC ABI; a layout mismatch on SysV (Linux) or AArch64 (Apple +/// Silicon) corrupts results silently rather than throwing, so the smoke +/// tests alone cannot catch it. +/// +/// +/// The expected values are derived from the C headers — compiled with +/// offsetof/sizeof — not from running the C# side. Asserting what +/// the managed code already does would prove nothing. +/// +/// +public class LayoutTests +{ + // ---- Ground truth from library/src/api/dll.h (offsetof/sizeof, LP64) ---- + + [Fact] + public void Deal_MatchesNativeLayout() + { + Assert.Equal(96, Marshal.SizeOf()); + Assert.Equal(0, (int) Marshal.OffsetOf(nameof(Deal.Trump))); + Assert.Equal(4, (int) Marshal.OffsetOf(nameof(Deal.First))); + Assert.Equal(8, (int) Marshal.OffsetOf(nameof(Deal.CurrentTrickSuit))); + Assert.Equal(20, (int) Marshal.OffsetOf(nameof(Deal.CurrentTrickRank))); + Assert.Equal(32, (int) Marshal.OffsetOf(nameof(Deal.RemainingCards))); + } + + [Fact] + public void FutureTricks_MatchesNativeLayout() + { + Assert.Equal(216, Marshal.SizeOf()); + Assert.Equal(0, (int) Marshal.OffsetOf(nameof(FutureTricks.Nodes))); + Assert.Equal(4, (int) Marshal.OffsetOf(nameof(FutureTricks.NumberOfCards))); + Assert.Equal(8, (int) Marshal.OffsetOf(nameof(FutureTricks.Suit))); + Assert.Equal(60, (int) Marshal.OffsetOf(nameof(FutureTricks.Ranks))); + Assert.Equal(112, (int) Marshal.OffsetOf(nameof(FutureTricks.EqualGroups))); + Assert.Equal(164, (int) Marshal.OffsetOf(nameof(FutureTricks.Score))); + } + + [Fact] + public void DdTableDeal_MatchesNativeLayout() + => Assert.Equal(64, Marshal.SizeOf()); + + [Fact] + public void DdTableResults_MatchesNativeLayout() + => Assert.Equal(80, Marshal.SizeOf()); + + [Fact] + public void ParResults_MatchesNativeLayout() + => Assert.Equal(288, Marshal.SizeOf()); + + [Fact] + public void DdTableDealPBN_MatchesNativeLayout() + => Assert.Equal(80, Marshal.SizeOf()); + + /// + /// remainCards is [hand][suit], row-major with DDS_SUITS = 4 columns, so + /// element [hand][suit] lives at index hand * 4 + suit. The JVM binding + /// relies on the same arithmetic; if FourHands ever disagreed, deals would + /// be silently transposed rather than rejected. + /// + [Fact] + public void FourHands_IsRowMajorByHandThenSuit() + { + var hands = new FourHands(); + for (int hand = 0; hand < 4; hand++) + for (int suit = 0; suit < 4; suit++) + hands[hand, suit] = (uint) (hand * 4 + suit); + + var flat = hands.AsSpan(); + for (int i = 0; i < FourHands.SIZE; i++) + Assert.Equal((uint) i, flat[i]); + } +} diff --git a/dotnet/DDS_Core.Tests/SmokeTests.cs b/dotnet/DDS_Core.Tests/SmokeTests.cs new file mode 100644 index 00000000..42862189 --- /dev/null +++ b/dotnet/DDS_Core.Tests/SmokeTests.cs @@ -0,0 +1,64 @@ +using DDS_Core; + +namespace DDS_Core.Tests; + +/// +/// End-to-end solving through the retargeted binding — the .NET analogue of +/// DdsSmokeTest.java. These are what prove the dds_c_* entry +/// points actually resolve and marshal correctly on a non-Windows platform. +/// +public class SmokeTests +{ + [Fact] + public void SolveBoard_ReferenceDeal_TakesThirteenTricks() + { + using var ctx = new SolverContext(); + + ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks fut); + + Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); + } + + [Fact] + public void CalcDdTable_ReferenceDeal_MatchesExpectedTable() + { + using var ctx = new SolverContext(); + var deal = TestDeals.ReferenceTable(); + + ctx.CalcDdTable(deal, out DdTableResults results); + + for (int strain = 0; strain < 5; strain++) + for (int hand = 0; hand < 4; hand++) + Assert.Equal(TestDeals.ExpectedDdTable[strain][hand], results.ResultsTable[strain, hand]); + } + + /// + /// The PBN twin must agree with the binary form. This is the only PBN pair + /// on the modern layer, added to the shim by this work. + /// + [Fact] + public void CalcDdTable_PbnAgreesWithBinary() + { + using var ctx = new SolverContext(); + + ctx.CalcDdTable(TestDeals.ReferenceTable(), out DdTableResults binary); + + var pbnDeal = new DdTableDealPBN { Cards = TestDeals.ReferencePbn }; + ctx.CalcDdTable(pbnDeal, out DdTableResults pbn); + + for (int strain = 0; strain < 5; strain++) + for (int hand = 0; hand < 4; hand++) + Assert.Equal(binary.ResultsTable[strain, hand], pbn.ResultsTable[strain, hand]); + } + + [Fact] + public void CalcPar_ReferenceDeal_ProducesNonEmptyScore() + { + using var ctx = new SolverContext(); + + ctx.CalcPar(TestDeals.ReferenceTable(), 0 /* vulnerable: none */, + out DdTableResults _, out ParResults par); + + Assert.False(string.IsNullOrWhiteSpace(par.ParScores[0])); + } +} diff --git a/dotnet/DDS_Core.Tests/TestDeals.cs b/dotnet/DDS_Core.Tests/TestDeals.cs new file mode 100644 index 00000000..d8f58fd3 --- /dev/null +++ b/dotnet/DDS_Core.Tests/TestDeals.cs @@ -0,0 +1,54 @@ +using DDS_Core; + +namespace DDS_Core.Tests; + +/// +/// The shared reference board, matching DdsSmokeTest.java and +/// dds_c_api_test.cpp so the JVM, C++, and .NET bindings all assert +/// against one fixture. +/// +internal static class TestDeals +{ + /// Full 13-card holding bitmask (ranks 2..A). + internal const uint FullSuit = 0x7FFC; + + /// + /// North holds all spades, East all hearts, South all diamonds, West all + /// clubs. With spades trump and North to lead, North/South take all 13. + /// + internal const int ExpectedTricks = 13; + + /// res_table[strain][hand] for the reference board. + internal static readonly int[][] ExpectedDdTable = + [ + [13, 0, 13, 0], // spades + [0, 13, 0, 13], // hearts + [13, 0, 13, 0], // diamonds + [0, 13, 0, 13], // clubs + [0, 0, 0, 0], // no-trump + ]; + + /// The same board in PBN: spades.hearts.diamonds.clubs per hand. + internal const string ReferencePbn = + "N:AKQJT98765432... .AKQJT98765432.. ..AKQJT98765432. ...AKQJT98765432"; + + internal static Deal Reference() + { + var deal = new Deal { Trump = 0, First = 0, RemainingCards = new FourHands() }; + deal.RemainingCards[0, 0] = FullSuit; // North spades + deal.RemainingCards[1, 1] = FullSuit; // East hearts + deal.RemainingCards[2, 2] = FullSuit; // South diamonds + deal.RemainingCards[3, 3] = FullSuit; // West clubs + return deal; + } + + internal static DdTableDeal ReferenceTable() + { + var deal = new DdTableDeal { Cards = new FourHands() }; + deal.Cards[0, 0] = FullSuit; + deal.Cards[1, 1] = FullSuit; + deal.Cards[2, 2] = FullSuit; + deal.Cards[3, 3] = FullSuit; + return deal; + } +} diff --git a/dotnet/DDS_Core/DDS_Core.slnx b/dotnet/DDS_Core/DDS_Core.slnx index c30e54c4..49f37b06 100644 --- a/dotnet/DDS_Core/DDS_Core.slnx +++ b/dotnet/DDS_Core/DDS_Core.slnx @@ -14,4 +14,10 @@ + + + + From e381e6f7672a9c0c17e6954222ce66586b2879e9 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 19:31:06 +0100 Subject: [PATCH 09/35] Add .NET binding coverage to Linux, Windows, and macOS CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows is the reason this is not deferred to the packaging follow-up. It was the only platform where the .NET binding worked, via dds_native.dll and dds_api.hpp's dds_* symbols; after the shim retarget it binds dds_c_* in the Bazel-built dds.dll, and no existing test covers that path — the JNI smoke tests and export_set_test are target_compatible_with-excluded on Windows. macOS is included rather than skipped because it is the only CI coverage of the AArch64 ABI, which is exactly what the managed struct-layout tests guard: a mismatch there corrupts results silently instead of throwing. With Linux (SysV x86-64) and Windows (Win64), all three ABIs are now exercised. Each job resolves the native library through `bazel info bazel-bin` rather than the bazel-bin convenience symlink, which is configuration-dependent and moves when a different --config is used, and fails with a clear message if the library is absent rather than letting dotnet report a confusing missing-entry-point error. The SDK is pinned to 8.0.x to match what DDS_Core targets; the test project's RollForward only takes effect where no 8.0 runtime exists, which is not the case on these runners. Verified locally on macOS/arm64 by running the workflow's exact command sequence: 19/19 tests pass. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci_linux.yml | 19 ++++++++++++++++++- .github/workflows/ci_macos.yml | 19 +++++++++++++++++++ .github/workflows/ci_windows.yml | 27 +++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index acf0d9fb..df172834 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -68,7 +68,24 @@ jobs: - name: Run all tests run: bazelisk test --verbose_failures //... - # 🔟 Upload test logs + # 🔟 .NET binding — build and test the managed wrapper against the shared + # library just built. Pinned to 8.0.x because that is what DDS_Core + # targets; the test project's RollForward only matters where no 8.0 + # runtime exists, which is not the case here. + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: dotnet-version: "8.0.x" + + # bazel-bin is a configuration-dependent convenience symlink, so ask Bazel + # for it rather than hardcoding a path that a different --config would move. + - name: Test .NET binding + run: | + bazel build --verbose_failures //jni:dds_shared + export DDS_LIBRARY_PATH="$(bazel info bazel-bin)/jni/libdds.so" + test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } + dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + + # 11 Upload test logs - name: Upload test logs - Linux if: always() uses: actions/upload-artifact@v6 diff --git a/.github/workflows/ci_macos.yml b/.github/workflows/ci_macos.yml index 11e91320..ca8a0447 100644 --- a/.github/workflows/ci_macos.yml +++ b/.github/workflows/ci_macos.yml @@ -49,6 +49,25 @@ jobs: - name: Run all tests run: bazelisk test --verbose_failures //... + # .NET binding — this job is the only CI coverage of the AArch64 ABI, which + # is what the managed struct-layout tests exist to guard: a layout mismatch + # there corrupts results silently rather than throwing. Linux covers SysV + # x86-64 and Windows covers Win64, so all three ABIs are exercised. + # Pinned to 8.0.x to match what DDS_Core targets. + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "8.0.x" + + # bazel-bin is a configuration-dependent convenience symlink, so ask Bazel + # for it rather than hardcoding a path that a different --config would move. + - name: Test .NET binding + run: | + bazelisk build --verbose_failures //jni:dds_shared + export DDS_LIBRARY_PATH="$(bazelisk info bazel-bin)/jni/libdds.dylib" + test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } + dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + # Upload test logs - name: Upload test logs - macOS if: always() diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index 94e97789..b0a55be7 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -56,6 +56,33 @@ jobs: - name: Run all tests run: bazelisk test --config=opt --verbose_failures //... + # .NET binding — this is the platform the shim retarget can regress. + # Windows was previously the only place the binding worked, via + # dds_native.dll and dds_api.hpp's dds_* symbols; it now binds dds_c_* + # in the Bazel-built dds.dll, a path no other test covers (the JNI smoke + # tests and export_set_test are target_compatible_with-excluded here). + # Pinned to 8.0.x to match what DDS_Core targets. + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "8.0.x" + + # bazel-bin is a configuration-dependent convenience symlink, so ask Bazel + # for it rather than hardcoding a path that a different --config would move. + - name: Test .NET binding + shell: pwsh + run: | + bazel build --verbose_failures //jni:dds_shared + if ($LASTEXITCODE -ne 0) { exit 1 } + $binDir = bazel info bazel-bin + $env:DDS_LIBRARY_PATH = Join-Path $binDir "jni\dds.dll" + if (-not (Test-Path $env:DDS_LIBRARY_PATH)) { + Write-Host "native library not found at $env:DDS_LIBRARY_PATH" + exit 1 + } + dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + # Upload test logs - name: Upload test logs - Windows if: always() From f82138d982c5a099fd147a3b478773a60150bd63 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sun, 19 Jul 2026 19:36:36 +0100 Subject: [PATCH 10/35] Update docs and specs for the .NET binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three claims in dds-public-api.md were falsified by this work and are corrected: the shim is no longer "a thin subset ... no *PBN twins and no TT configure"; .NET no longer P/Invokes dds_* from dds_api.hpp; and the PBN pairing now does extend to the shim, via dds_c_calc_dd_table_pbn. jni-ffm-binding.md notes that the Windows DLLEXPORT superset is no longer relied on by any shipped binding, and that //jni:dds_shared serves .NET as well despite its location. Adds specs/dotnet-binding.md, a capability spec the repo lacked despite having one for the Python and JVM bindings. It records which ABI layer is bound and why, the single "dds" library name, DDS_LIBRARY_PATH resolution, SafeHandle ownership and one-context-per-thread, and the invariants the layout, smoke, and lifecycle tests enforce. Capability-level only — no per-method signatures, which stay in doxygen and docs/. docs/dotnet_interface.md gains a build-and-load section covering the Bazel target, per-OS artifacts, both load paths, and the advice to resolve via `bazel info bazel-bin` rather than the configuration-dependent symlink. It also states which native symbols are used, since binding dds_api.hpp is what previously made the wrapper Windows-only. Co-Authored-By: Claude Opus 4.8 --- docs/dotnet_interface.md | 66 +++++++++++++++++++++++++ docs/jni_interface.md | 16 +++--- specs/dds-public-api.md | 37 ++++++++------ specs/dotnet-binding.md | 103 +++++++++++++++++++++++++++++++++++++++ specs/jni-ffm-binding.md | 11 +++-- 5 files changed, 210 insertions(+), 23 deletions(-) create mode 100644 specs/dotnet-binding.md diff --git a/docs/dotnet_interface.md b/docs/dotnet_interface.md index 573f9381..75d24a4a 100644 --- a/docs/dotnet_interface.md +++ b/docs/dotnet_interface.md @@ -9,9 +9,16 @@ The library exposes the full DDS functionality through idiomatic `.Net` structur The goal is to provide a stable, fast, and fully documented .NET interface to the DDS engine. +It runs on **macOS, Linux, and Windows** from the same assembly — see +[Building and loading the native library](#building-and-loading-the-native-library). +The bindings mirror how the other language bindings work; see +[jni_interface.md](jni_interface.md), [python_interface.md](python_interface.md), +and [wasm_build.md](wasm_build.md). + --- ## Table of Contents +0. [Building and loading the native library](#building-and-loading-the-native-library) 1. [Introduction](#introduction) 2. [Legacy vs. Modern DDS API](#legacy-vs-modern-dds-api) 3. [Basic Usage](#basic-usage) @@ -39,6 +46,65 @@ The goal is to provide a stable, fast, and fully documented .NET interface to th --- +# Building and loading the native library + +`DDS_Core` is a managed wrapper; the solver itself lives in one self-contained +native library built by Bazel: + +```bash +bazel build //jni:dds_shared +``` + +| OS | Artifact | Location | +| ------- | -------------- | ---------------- | +| Linux | `libdds.so` | `bazel-bin/jni/` | +| macOS | `libdds.dylib` | `bazel-bin/jni/` | +| Windows | `dds.dll` | `bazel-bin/jni/` | + +Despite living under `//jni`, this is the shared native artifact for every +binding — the JVM one uses it too. + +## Finding the library at runtime + +The binding imports the library under the single name **`dds`**; .NET's probing +supplies the `lib` prefix and the per-OS extension, so one name resolves all +three artifacts above. There are two ways to point it at a library: + +- **Default probing** — place the library where the runtime already looks (next + to the application, or under `runtimes//native/` in a package). No + configuration needed. +- **`DDS_LIBRARY_PATH`** — set this environment variable to the *full path* of a + library file to override probing. This is the counterpart of the JVM binding's + `-Ddds.library.path`, and is how the tests bind against a freshly-built + library: + + ```bash + export DDS_LIBRARY_PATH="$(bazel info bazel-bin)/jni/libdds.dylib" + dotnet test dotnet/DDS_Core.Tests/ + ``` + + Use `bazel info bazel-bin` rather than the `bazel-bin` symlink: that symlink is + configuration-dependent and moves when you build with a different `--config`. + If the variable is set but the library cannot be loaded, the error names the + path you gave rather than reporting a missing entry point later. + +## Which native symbols are used + +The modern context API binds the pure-C shim (`dds_c_*`, from +`library/src/api/dds_c_api.h`); the legacy flat API binds `dll.h` directly. The +reference-taking `dds_*` functions in `dds_api.hpp` are deliberately **not** +used — they are not exported on Linux or macOS, and binding them is what +previously made this wrapper Windows-only. + +## Requirements + +.NET 8.0 or later (`.NET Framework` is not supported). If you build the test +project on a machine that has only a newer major runtime installed, note it sets +`Major` so it still runs; the library itself targets +`net8.0`. + +--- + # Introduction `DDS_Core` provides a managed .NET interface to the DDS engine. diff --git a/docs/jni_interface.md b/docs/jni_interface.md index 301911bf..2a08c34d 100644 --- a/docs/jni_interface.md +++ b/docs/jni_interface.md @@ -72,12 +72,16 @@ and ctypes can all bind to: - The solver handle is opaque: `typedef void* DDS_C_SOLVER_CTX`. - Every struct is passed by pointer; no non-POD C++ type crosses the boundary. - -Shim entry points: `dds_c_create_solvercontext_default`, -`dds_c_destroy_solvercontext`, `dds_c_solve_board`, `dds_c_calc_dd_table`, -`dds_c_calc_par`. The flat legacy C API from `dll.h` (`SolveBoard`, -`CalcDDtable`, `GetDDSInfo`, `ErrorMessage`, …) is exported unchanged and is -also callable from FFM. + `SolverConfig` is decomposed into scalar arguments and `TTKind` crosses as an + `int`, so nothing is passed by value either. + +The shim covers the modern API's full surface: context lifecycle (default and +config-based creation, destroy), `dds_c_solve_board`, `dds_c_calc_dd_table` and +its `_pbn` twin, `dds_c_calc_par`, transposition-table configure/resize/clear, +both resets, and the logging passthroughs. The Java bindings here use a subset; +the .NET binding uses all of it ([dotnet_interface.md](dotnet_interface.md)). +The flat legacy C API from `dll.h` (`SolveBoard`, `CalcDDtable`, `GetDDSInfo`, +`ErrorMessage`, …) is exported unchanged and is also callable from FFM. ## Using the FFM bindings diff --git a/specs/dds-public-api.md b/specs/dds-public-api.md index da023ff0..325f0157 100644 --- a/specs/dds-public-api.md +++ b/specs/dds-public-api.md @@ -1,7 +1,7 @@ --- capability: dds-public-api owners: [api] -last-updated: 2026-07-18 +last-updated: 2026-07-19 --- # DDS Public API @@ -36,17 +36,21 @@ capability defines what crosses the boundary and promises to stay stable. header for the full set. 3. **Pure-C ABI shim** — `dds_c_api.h` (`dds_c_*`). Pointer-only, POD-only, opaque `void*` handle (`DDS_C_SOLVER_CTX`); no C++ types cross the boundary. - It forwards to layer 2. Today it is a thin subset (`dds_c_solve_board`, - `dds_c_calc_dd_table`, `dds_c_calc_par` plus context lifecycle) — **no - `*PBN` twins and no TT configure** in the shim. -- **Bindings pick different layers.** Java/FFM binds the shim (`dds_c_*` plus - `GetDDSInfo` from `dll.h`) — see [jni-ffm-binding](jni-ffm-binding.md). .NET - P/Invokes the modern `dds_*` symbols from `dds_api.hpp` - (`dotnet/DDS_Core/Native/DdsNative.cs`). Python wraps the C++ API via pybind11 - ([python-binding](python-binding.md)), not the C shim. There is no shipped - ctypes binding. The shim header is C-ABI but not C-includable (it pulls in - `dll.h`, which uses C++ trailing-return syntax) — bind to compiled symbols or - parse in C++ mode (jextract). + It forwards to layer 2 and now covers that layer's full surface: context + lifecycle (including config-based creation), `dds_c_solve_board`, + `dds_c_calc_dd_table` and its `_pbn` twin, `dds_c_calc_par`, TT + configure/resize/clear, both resets, and the logging passthroughs. + `SolverConfig` is decomposed into scalar arguments and `TTKind` crosses as + an `int`, so no struct is passed by value. +- **Bindings pick different layers.** Java/FFM and .NET both bind the shim + (`dds_c_*`) plus the flat `dll.h` API — see + [jni-ffm-binding](jni-ffm-binding.md) and [dotnet-binding](dotnet-binding.md). + .NET reaches the shim through `EntryPoint` on its P/Invokes + (`dotnet/DDS_Core/Native/DdsNative.cs`), keeping its managed method names. + Python wraps the C++ API via pybind11 ([python-binding](python-binding.md)), + not the C shim. There is no shipped ctypes binding. The shim header is C-ABI + but not C-includable (it pulls in `dll.h`, which uses C++ trailing-return + syntax) — bind to compiled symbols or parse in C++ mode (jextract). - **Handles are single-threaded.** One `DDS_SOLVER_CTX` / `DDS_C_SOLVER_CTX` per thread; the handle owns per-context solver state and its transposition table. Create → use → destroy. The legacy flat API manages global/threaded state via @@ -57,13 +61,17 @@ capability defines what crosses the boundary and promises to stay stable. entry points have a `*PBN` twin; both compute identical results from the same deal. On the modern C++ layer only `dds_calc_dd_table` has one (`dds_calc_dd_table_pbn`) — `dds_solve_board` and `dds_calc_par` do not. The - pairing does **not** extend to the C shim. + shim mirrors the modern layer exactly: `dds_c_calc_dd_table_pbn` is its only + PBN twin. Agreement between each pair is asserted by + `//library/tests:dds_c_api_test` and by the .NET smoke tests. - **The pinned binding export set is `dll.h` + `dds_c_api.h`.** On Linux/macOS the JNI shared library exports are constrained by `jni/version_script.lds` / `exported_symbols.lds` and checked by the export-set test. That is the *stable binding ABI*, not every `DLLEXPORT` symbol in the tree: `dds_api.hpp` also marks modern `dds_*` symbols `DLLEXPORT`, and on Windows (no `.lds`) the DLL exports that broader `DLLEXPORT` set. Details in [jni-ffm-binding](jni-ffm-binding.md). + Now that the shim covers the whole modern surface, no shipped binding depends + on that Windows-only surplus: all three platforms bind the same pinned set. ## Key entry points @@ -71,7 +79,8 @@ capability defines what crosses the boundary and promises to stay stable. narrative in `docs/legacy_c_api.md`. - `library/src/api/dds_api.hpp` — modern context C++ API. Narrative in `docs/c++_interface.md`; migration in `docs/api_migration.md`. -- `library/src/api/dds_c_api.h` — pure-C ABI shim (Java/FFM binding surface). +- `library/src/api/dds_c_api.h` — pure-C ABI shim (the Java/FFM and .NET binding + surface). Guarded by `//library/tests:dds_c_api_test`. - `library/src/api/{solve_board,calc_dd_table,calc_par}.hpp`, `PBN.h`, `portab.h`, `dds.h` — supporting public headers (`api_definitions`). - Build targets: `//library/src/api:dds_c_api`, `:api_definitions`, `//:dds` diff --git a/specs/dotnet-binding.md b/specs/dotnet-binding.md new file mode 100644 index 00000000..f5804684 --- /dev/null +++ b/specs/dotnet-binding.md @@ -0,0 +1,103 @@ +--- +capability: dotnet-binding +owners: [dotnet] +last-updated: 2026-07-19 +--- + +# .NET Binding (DDS_Core) + +> **Specs vs. doxygen / docs.** How to call the wrapper — the type-by-type API, +> legacy-vs-modern guidance, worked examples — is in `docs/dotnet_interface.md` +> and the XML doc comments on the types themselves. This spec records the +> cross-cutting contracts: which ABI layer is bound, how the native library is +> found, and the invariants the tests enforce. + +## Purpose + +This capability lets .NET consumers call the solver through an idiomatic managed +API — blittable structs, `SafeHandle` lifetimes, method overloading — on macOS, +Linux, and Windows from the same assembly. It exists so a .NET application gets +the solver without building the C++ or writing marshalling code. Like the JVM +binding it targets the pure-C shim from [dds-public-api](dds-public-api.md), not +the C++ API. + +## Behaviour & invariants + +> Per-type and per-method detail is in `DDS_Core`'s doc comments and +> `docs/dotnet_interface.md`. These are the whole-binding guarantees. + +- **Two ABI layers, one library.** The modern context entry points bind the + `dds_c_*` shim; the legacy flat API (`SolveBoard`, `CalcDDtable`, `Par`, + `Analyse*`, …) binds `dll.h` directly. Both come from the single native + library built by `//jni:dds_shared`. The binding does **not** use the + reference-taking `dds_*` symbols in `dds_api.hpp`: those are not exported on + Linux or macOS, so binding them made the wrapper Windows-only. +- **Managed names are decoupled from ABI names.** The shim is reached via + `EntryPoint` on each `DllImport`, so `DdsNative`'s method names — and every + call site in `DDS.cs` / `SolverContext.cs` — are independent of the C symbol + names. Renaming a shim export touches one attribute. +- **No struct crosses the boundary by value.** `SolverConfig` is unpacked into + scalar arguments at the P/Invoke and `TTKind` marshals as its underlying + `int`, matching the shim's pointer-only, POD-only contract. +- **One native library name: `dds`.** .NET's probing supplies the `lib` prefix + and per-OS extension, so a single `DllName` resolves `libdds.dylib`, + `libdds.so`, and `dds.dll`. +- **Library resolution is overridable.** A resolver registered from + `DdsNative`'s static constructor — which the runtime guarantees runs before + that type's first P/Invoke — honours the `DDS_LIBRARY_PATH` environment + variable, falling back to default probing when it is unset. When it is set but + unusable the failure names the attempted path rather than silently falling + through. This is the counterpart of the JVM binding's `-Ddds.library.path` and + is intended for tests and development, not deployment. +- **Struct layouts are pinned by tests, not by convention.** The managed structs + must match the C structs in `dll.h` byte for byte; a mismatch on SysV or + AArch64 corrupts results *silently* rather than throwing. `LayoutTests` + asserts sizes and field offsets against values derived from the C headers, and + `FourHands` indexing against the `hand * 4 + suit` row-major layout the other + bindings assume. +- **Solver contexts are single-threaded and deterministically released.** + `SolverContext` owns a `SolverContextHandle` (`SafeHandle`), so the native + context is freed on `Dispose` or finalization; disposal is idempotent. One + context per thread, as with every binding (see + [solver-context](solver-context.md)). +- **Integer status returns.** Entry points return `RETURN_*` codes as elsewhere + in the API; the wrapper converts failures to exceptions at its public surface. +- **All three ABIs are covered by CI.** The managed tests run on Linux (SysV + x86-64), Windows (Win64), and macOS (AArch64), each against the Bazel-built + shared library located via `bazel info bazel-bin`. Windows matters most: it is + the platform the shim retarget could regress, and the Bazel-built `dds.dll` + path is covered by no other test, since the JNI tests are + `target_compatible_with`-excluded there. + +## Key entry points + +- `dotnet/DDS_Core/Native/DdsNative.cs` — every P/Invoke; `DllName` and the + `EntryPoint` mapping onto `dds_c_*`. +- `dotnet/DDS_Core/Native/DdsNativeResolver.cs` — `DDS_LIBRARY_PATH` resolution. +- `dotnet/DDS_Core/DataModel/SolverContext.cs` — the modern managed API; + `Helpers/SolverContextHandle.cs` — `SafeHandle` ownership. +- `dotnet/DDS_Core/DataModel/`, `Helpers/` — the blittable structs and inline + array helpers whose layouts the tests pin. +- Native artifact: `//jni:dds_shared` (see [jni-ffm-binding](jni-ffm-binding.md)). +- Consumer guide: `docs/dotnet_interface.md`. +- Guarded by `dotnet/DDS_Core.Tests/` (`LayoutTests`, `SmokeTests`, + `ContextLifecycleTests`) and, on the native side, + `//library/tests:dds_c_api_test`. + +## Known gaps / non-goals + +- **No NuGet package yet.** Consumers build the project and supply the native + library themselves. RID-specific packaging (`runtimes//native/…`) and a + multi-platform package are a deliberate follow-up, mirroring how the jar + followed the JVM shared library. +- **`solution/dds_native.vcxproj` is no longer the shipped native artifact** but + is still present, and `DDS_Core.slnx` still build-depends on it. It builds a + differently-named DLL that the binding no longer loads; retiring it is + deferred. As a consequence `DDS_Core.slnx` does not build under `dotnet + build` — it includes a C++ project needing Visual Studio's MSBuild. The + individual projects build fine, which is what CI uses. +- **The test project targets `net8.0` with `RollForward` set to `Major`**, so it + also runs where only a newer major runtime is installed. CI pins an 8.0 SDK, + where the property has no effect. +- Per-type API documentation is intentionally not duplicated here; it lives in + `docs/dotnet_interface.md` and the types' doc comments. diff --git a/specs/jni-ffm-binding.md b/specs/jni-ffm-binding.md index 6b2f5ac0..c98866f3 100644 --- a/specs/jni-ffm-binding.md +++ b/specs/jni-ffm-binding.md @@ -1,7 +1,7 @@ --- capability: jni-ffm-binding owners: [jni] -last-updated: 2026-07-18 +last-updated: 2026-07-19 --- # JVM Binding (Foreign Function & Memory) @@ -30,7 +30,9 @@ shim from [dds-public-api](dds-public-api.md), not the C++ API. links every internal sub-library statically so `System.loadLibrary("dds")` needs exactly one file. Per-OS name: `dds.dll` / `libdds.dylib` / `libdds.so`. On Unix the export set is the stable C ABI (`dll.h` + `dds_c_*`); on Windows - it is broader — see the next bullet. + it is broader — see the next bullet. Despite living under `//jni`, this target + is the shared native artifact for the .NET binding too + ([dotnet-binding](dotnet-binding.md)). - **The exported ABI is pinned by checked-in export lists on Unix.** Linux links with `version_script.lds` (`-Wl,--version-script`), macOS with `exported_symbols.lds` (`-Wl,-exported_symbols_list`). Those `.lds` files are @@ -40,7 +42,10 @@ shim from [dds-public-api](dds-public-api.md), not the C++ API. parser is unit-tested by `gen_export_lists_test`. **Windows has no `.lds` branch:** the DLL exports whatever is marked `DLLEXPORT`, which is a **superset** of the Unix list (it also includes the modern `dds_*` context API - from `dds_api.hpp`). See [dds-public-api](dds-public-api.md). + from `dds_api.hpp`). No shipped binding relies on that surplus any more: the + shim now covers the whole modern surface, so the JVM and .NET bindings bind + the same pinned set on every platform. See + [dds-public-api](dds-public-api.md) and [dotnet-binding](dotnet-binding.md). - **Bindings are hand-written, not jextract-generated.** `//jni:dds_ffm` (`java_library`, `java/org/dds/ffm/Dds.java`) declares the struct `MemoryLayout`s and `Linker` downcall handles for the `dds_c_*` shim plus From 14c302ec04957c4da02059662de6c91987328ae9 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 15:58:24 +0100 Subject: [PATCH 11/35] Fix heap-use-after-free in clear_tt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clear_tt() returned the transposition table's memory but kept the instance. SearchContext::trans_table() only checks whether tt_ is non-null, so it handed that husk straight back and the next lookup read freed pools. Caught by ASan on Linux CI: allocated: TransTable{L,S}::make_tt freed: TransTable{L,S}::return_all_memory <- clear_tt() read: TransTable{L,S}::lookup <- next solve Disposing the instance instead makes the documented "recreates lazily on demand" behaviour real: tt_ becomes null, so the next trans_table() rebuilds from the owner's config. Nothing is lost, since the kind and memory limits live in SolverContext::cfg_ rather than in the TT instance, and the destructor returns the memory either way. Disposal also keeps the fix off the hot path — ab_search calls trans_table() per lookup, so re-checking allocation state there would have cost more than it saved. This is pre-existing and independent of the .NET work: reproduced through the reference-taking C++ API as dds_clear_tt() followed by dds_solve_board(), with no dds_c_* call involved. It affects the default Large TT, so the existing .NET binding has the same latent fault via SolverContext.ClearTT() then SolveBoard(). Note this supersedes the TransTableS::reset_memory guard added earlier in this branch as the active fix for that path: with the instance disposed, maybe_trans_table() returns null and reset_for_solve() no longer reaches a memory-less table. That guard is left in place as defence in depth, mirroring the one TransTableL::reset_memory already had. Verified: ASan and TSan clean across //library/tests/..., 59/59 Bazel tests, 19/19 .NET tests. Co-Authored-By: Claude Opus 4.8 --- library/src/solver_context/solver_context.cpp | 14 ++++++++++++-- library/src/solver_context/solver_context.hpp | 9 +++++++-- library/tests/dds_c_api_test.cpp | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/library/src/solver_context/solver_context.cpp b/library/src/solver_context/solver_context.cpp index 6b2ee5af..ac8f0385 100644 --- a/library/src/solver_context/solver_context.cpp +++ b/library/src/solver_context/solver_context.cpp @@ -200,8 +200,18 @@ auto SolverContext::clear_tt() const -> void #ifdef DDS_UTILITIES_LOG utilities().log_append("tt:clear"); #endif - if (auto* tt = search_.maybe_trans_table()) - tt->return_all_memory(); + // Dispose the instance rather than calling return_all_memory() on it. Both + // free the pools — the TT destructor returns all memory — but returning the + // memory while keeping the object leaves a husk whose pool pointers dangle, + // and SearchContext::trans_table() hands that husk straight back because it + // only checks whether tt_ is non-null. The next lookup then reads freed + // memory (ASan: heap-use-after-free in TransTable{L,S}::lookup). + // + // Disposing instead makes the documented "recreates lazily on demand" + // behaviour real: tt_ becomes null, so the next trans_table() rebuilds from + // the owner's config. Nothing is lost, because the kind and memory limits + // live in SolverContext::cfg_, not in the TT instance. + const_cast(this)->search_.dispose_trans_table(); } auto SolverContext::resize_tt(int defMB, int maxMB) const -> void diff --git a/library/src/solver_context/solver_context.hpp b/library/src/solver_context/solver_context.hpp index 3c7124de..1c63b7fd 100644 --- a/library/src/solver_context/solver_context.hpp +++ b/library/src/solver_context/solver_context.hpp @@ -216,9 +216,14 @@ class SolverContext */ auto reset_best_moves_lite() const -> void; /** - * @brief Return all TT memory to the system without destroying the TT. + * @brief Return all TT memory to the system. + * + * Disposes the TT instance; the configured kind and memory limits persist on + * the context, so the next use recreates an empty table from them. Keeping a + * memory-less instance alive instead would leave dangling pool pointers for + * the next lookup to read. */ - auto clear_tt() const -> void; // Calls ReturnAllMemory() + auto clear_tt() const -> void; /** * @brief Resize TT memory defaults and limits in-place if TT exists. */ diff --git a/library/tests/dds_c_api_test.cpp b/library/tests/dds_c_api_test.cpp index dc51322f..4258bc33 100644 --- a/library/tests/dds_c_api_test.cpp +++ b/library/tests/dds_c_api_test.cpp @@ -168,6 +168,23 @@ TEST(DdsCApiTtConfiguration, ContextRemainsUsableAfterReconfiguration) dds_c_destroy_solvercontext(ctx); } +// Regression: clear_tt() used to return the TT's memory while keeping the +// instance, so the next lookup read freed pools (ASan: heap-use-after-free in +// TransTableL::lookup_suit). This is the default-configuration path — no TT +// kind switch involved — and is reachable from every binding as +// clear_tt() followed by a solve. +TEST(DdsCApiTtConfiguration, ClearTtThenSolveOnDefaultTt) +{ + DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); + ASSERT_NE(ctx, nullptr); + + ASSERT_EQ(SolveReference(ctx), kExpectedTricks); + dds_c_clear_tt(ctx); + EXPECT_EQ(SolveReference(ctx), kExpectedTricks); + + dds_c_destroy_solvercontext(ctx); +} + TEST(DdsCApiResets, ResetsLeaveContextUsable) { DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); From f855fbef09db849c7fa0d33b6bc75a478f187fd9 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 19:36:24 +0100 Subject: [PATCH 12/35] Route clear_tt through dispose_trans_table clear_tt() performed the disposal with a raw const_cast on search_, bypassing SolverContext::dispose_trans_table() twelve lines above. That skipped the "tt:dispose" log entry and the tt_disposes stats counter, so the counter under-reported and the log trace no longer recorded that the TT went away. configure_tt() already calls dispose_trans_table() for the same operation; the two disposal paths are now consistent. Co-Authored-By: Claude Opus 4.8 --- library/src/solver_context/solver_context.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/src/solver_context/solver_context.cpp b/library/src/solver_context/solver_context.cpp index ac8f0385..1370949e 100644 --- a/library/src/solver_context/solver_context.cpp +++ b/library/src/solver_context/solver_context.cpp @@ -211,7 +211,7 @@ auto SolverContext::clear_tt() const -> void // behaviour real: tt_ becomes null, so the next trans_table() rebuilds from // the owner's config. Nothing is lost, because the kind and memory limits // live in SolverContext::cfg_, not in the TT instance. - const_cast(this)->search_.dispose_trans_table(); + dispose_trans_table(); } auto SolverContext::resize_tt(int defMB, int maxMB) const -> void From bc7df8271947c4c6ec7b9bb6110484703666ba7d Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 19:40:52 +0100 Subject: [PATCH 13/35] Amend solver-context spec for the new clear_tt semantics The heap-use-after-free fix changed clear_tt() from return_all_memory() on the live TT to disposing the instance, but the spec still documented the old behaviour in two places -- including an explicit "reuse after clear_tt() is unsafe" warning and "No test guards this today", both of which are now false. A caller reading the spec would work around a hazard that no longer exists and treat a supported path as broken. Co-Authored-By: Claude Opus 4.8 --- specs/solver-context.md | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/specs/solver-context.md b/specs/solver-context.md index c2bf5c08..6ab48b08 100644 --- a/specs/solver-context.md +++ b/specs/solver-context.md @@ -1,7 +1,7 @@ --- capability: solver-context owners: [solver_context] -last-updated: 2026-07-18 +last-updated: 2026-07-20 --- # Solver Context @@ -48,19 +48,19 @@ the opaque handle. See [dds-public-api](dds-public-api.md). `reset_for_solve()` clears a subset of search state and resets TT memory (`ResetReason::FreeMemory`) while preserving the allocation for reuse; `reset_best_moves_lite()` clears only best-move ranks (hot per-iteration path); - `clear_tt()` calls `return_all_memory()` on the existing TT object; - `dispose_trans_table()` destroys the TT immediately. -- **`clear_tt()` does not leave a reusable table.** It keeps the `unique_ptr` - alive but frees the table's storage, and nothing re-runs `make_tt()`: - `SearchContext::trans_table()` returns early whenever `tt_` is non-null, and - `TransTable::init()` only fills aggregate lookup arrays — it does not - reallocate. Reusing the context after `clear_tt()` without - `dispose_trans_table()` (or recreating the context) is **unsafe**: the next - `lookup`/`add` can touch freed memory. Do not treat this as a quiet "dead - cache" — `TransTableS` does not gate `lookup`/`add` on `tt_in_use_`, and - `TransTableL::return_all_memory()` does not put the table into a reliably - inert state. Use `dispose_trans_table()` when the next solve should get a - fresh table. No test guards this today. + `clear_tt()` disposes the TT instance; `dispose_trans_table()` destroys the + TT immediately. +- **`clear_tt()` leaves the context reusable.** It disposes the TT instance + rather than calling `return_all_memory()` on it, so `tt_` becomes null and + the next `SearchContext::trans_table()` rebuilds an empty table lazily. The + configured kind and memory limits survive, because they live in + `SolverContext::cfg_` and not in the TT instance. Calling `return_all_memory()` + while keeping the object was the earlier behaviour and was **unsafe**: it left + a husk whose pool pointers dangled, which `trans_table()` handed straight back + because it only checks whether `tt_` is non-null — the next `lookup`/`add` + read freed memory. `clear_tt()` and `dispose_trans_table()` now differ only in + their log/stats trace. Guarded by `//library/tests:dds_c_api_test` + (`DdsCApiTtConfiguration.ClearTtThenSolveOnDefaultTt`). - **Hot-path facades are value-typed and inline-friendly, with different holds.** `MoveGenContext` holds a raw `ThreadData*` so `move_gen()` can return a value-typed facade without an atomic `shared_ptr` bump on every call. From 1562709aab1d2a448bd79db06d4604b601b3106e Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 20:03:49 +0100 Subject: [PATCH 14/35] Cover the TransTableS reset guard with a test that reaches it Two tests claimed to guard TransTableS::reset_memory()'s tt_in_use_ early return, but neither reached it: clear_tt() now disposes the TT, so reset_for_solve() sees no table and never calls reset_memory(). Both would have passed with the guard reverted -- verified by temporarily removing it. Add a direct unit test driving TransTableS through make_tt/return_all_memory/reset_memory, which segfaults without the guard (verified), and reword the two indirect tests plus the guard's comment to describe what they actually cover. Co-Authored-By: Claude Opus 4.8 --- .../DDS_Core.Tests/ContextLifecycleTests.cs | 13 +++---- library/src/trans_table/trans_table_s.cpp | 9 +++-- library/tests/dds_c_api_test.cpp | 10 +++--- .../tests/trans_table/trans_table_s_test.cpp | 36 ++++++++++++++++++- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs index 202a759e..40630f14 100644 --- a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs +++ b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs @@ -53,14 +53,15 @@ public void Resets_LeaveContextUsable() } /// - /// Regression for the Small-TT crash fixed alongside this binding work: - /// ClearTT() released the transposition-table pools and a following - /// ResetForSolve() re-initialised over them, faulting inside - /// TransTableS::init_tt(). Reachable from managed code exactly as written - /// here, so this is the .NET-side guard for that fix. + /// A Small-TT context survives ClearTT() followed by ResetForSolve() and + /// still solves. ClearTT() disposes the transposition table, so the + /// following solve rebuilds one lazily from the context's configuration. + /// This exercises the managed TT-lifecycle path; the native + /// TransTableS::reset_memory() guard is covered by + /// //library/tests/trans_table:trans_table. /// [Fact] - public void SmallTt_ClearThenResetForSolve_DoesNotCrash() + public void SmallTt_ClearThenResetForSolve_StillSolves() { using var ctx = new SolverContext(); ctx.SolveBoard(TestDeals.Reference(), -1, 1, 1, out FutureTricks _); diff --git a/library/src/trans_table/trans_table_s.cpp b/library/src/trans_table/trans_table_s.cpp index eaa57240..73f92ab2 100644 --- a/library/src/trans_table/trans_table_s.cpp +++ b/library/src/trans_table/trans_table_s.cpp @@ -346,9 +346,12 @@ auto TransTableS::reset_memory( // Nothing to reset when the pools have been returned: return_all_memory() // frees pw_/pn_/pl_ and clears tt_in_use_, and make_tt() reallocates lazily // before the next lookup. Without this guard init_tt() below dereferences - // the freed pools (pw_[0]) and segfaults — reachable from the public API as - // configure_tt(Small) -> clear_tt() -> reset_for_solve(). TransTableL's - // reset_memory() already guards the equivalent case with `pool_ == nullptr`. + // the freed pools (pw_[0]) and segfaults. TransTableL's reset_memory() + // already guards the equivalent case with `pool_ == nullptr`. + // + // Defensive: SolverContext::clear_tt() disposes the TT instance rather than + // returning its memory, so no public-API sequence reaches this today. It is + // covered directly by TransTableSMemoryTest.ResetAfterReturnAllMemoryIsInert. if (!tt_in_use_) return; diff --git a/library/tests/dds_c_api_test.cpp b/library/tests/dds_c_api_test.cpp index 4258bc33..fb64cc38 100644 --- a/library/tests/dds_c_api_test.cpp +++ b/library/tests/dds_c_api_test.cpp @@ -199,11 +199,11 @@ TEST(DdsCApiResets, ResetsLeaveContextUsable) dds_c_destroy_solvercontext(ctx); } -// Regression: on a Small TT, clear_tt() returns the pools and a following -// reset_for_solve() used to re-init over them, dereferencing null in -// TransTableS::init_tt(). The Large TT (the default) was never affected -// because TransTableL::reset_memory() already guarded the equivalent case. -// Reachable from the public API, so this covers the C++ and .NET paths too. +// A Small-TT context survives clear_tt() followed by reset_for_solve() and +// still solves. clear_tt() disposes the TT, so reset_for_solve() finds no +// table and the following solve rebuilds one lazily from the context's config. +// The TransTableS::reset_memory() guard is not on this path — it is covered +// directly by //library/tests/trans_table:trans_table. TEST(DdsCApiTtConfiguration, SmallTtClearThenResetForSolve) { DDS_C_SOLVER_CTX ctx = dds_c_create_solvercontext_default(); diff --git a/library/tests/trans_table/trans_table_s_test.cpp b/library/tests/trans_table/trans_table_s_test.cpp index f0e95008..a0239304 100644 --- a/library/tests/trans_table/trans_table_s_test.cpp +++ b/library/tests/trans_table/trans_table_s_test.cpp @@ -4,7 +4,7 @@ // Include DDS types first #include -// No TransTable dependencies needed in this file; remove legacy forward declarations. +#include namespace dds_test { @@ -33,6 +33,40 @@ static void CreateTestWinRanks(unsigned short win_ranks[DDS_SUITS]) { win_ranks[3] = 0x8888; // Clubs } +// Regression: reset_memory() after return_all_memory() must be inert. +// return_all_memory() frees pw_/pn_/pl_ and clears tt_in_use_; without the +// guard in TransTableS::reset_memory(), init_tt() dereferences the freed pools +// (pw_[0]) and segfaults. TransTableL::reset_memory() already guards the +// equivalent case with `pool_ == nullptr`. +TEST(TransTableSMemoryTest, ResetAfterReturnAllMemoryIsInert) { + TransTableS tt; + + tt.set_memory_maximum(1); + tt.make_tt(); + tt.return_all_memory(); + + // The guard under test. Without it this call segfaults: init_tt() + // dereferences pw_[0], which return_all_memory() has already freed. + // Reaching this line at all is the regression assertion. + tt.reset_memory(ResetReason::FreeMemory); + + // The table is usable again once the pools are reallocated. + tt.make_tt(); + + int handLookup[15][15]; + CreateBasicHandLookup(handLookup); + tt.init(handLookup); + tt.reset_memory(ResetReason::NewDeal); + + unsigned short aggrTarget[DDS_SUITS]; + CreateTestAggrTarget(aggrTarget); + int hand_dist[4] = {0, 0, 0, 0}; + bool lowerFlag = false; + + // Nothing was added, so the rebuilt table must miss rather than crash. + EXPECT_EQ(tt.lookup(1, 0, aggrTarget, hand_dist, 0, lowerFlag), nullptr); +} + // Test that verifies DDS constants are available TEST(TransTableSBasicTest, DDSConstantsAvailable) { // Verify that basic DDS constants are accessible From d6f2769e0d8a2c642fde7295f11066c98482c736 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 20:04:35 +0100 Subject: [PATCH 15/35] Correct the dds_c_api_test dependency comment The comment claimed the //library/src:dds edge retains an __attribute__((constructor)) that initializes static solver memory. All three parts were wrong: on Windows dds.cpp takes the DllMain branch, and DllMain never runs for a statically-linked cc_test; a plain deps edge does not force retention of an unreferenced object file (that needs alwayslink); and dds_c_api already depends on //library/src:dds, so the edge changes nothing. The cited failure mode belongs to the TransTableS reset guard, which is unrelated to static-memory init. The test passes because SolverContext(SolverConfig) allocates its own ThreadData. Say that instead. Co-Authored-By: Claude Opus 4.8 --- library/tests/BUILD.bazel | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/library/tests/BUILD.bazel b/library/tests/BUILD.bazel index 74d0c171..f2768223 100644 --- a/library/tests/BUILD.bazel +++ b/library/tests/BUILD.bazel @@ -79,11 +79,12 @@ cc_test( # catch-all wrappers that exist only at that boundary and would be bypassed by # calling the reference-taking dds_* functions directly. # -# Depends on //library/src:dds explicitly, not just transitively through -# dds_c_api: dds.cpp carries the __attribute__((constructor)) that initializes -# static solver memory, and without a direct edge the linker drops that object -# file — leaving TransTableS::init_tt() to dereference null on the first solve -# after a TT-kind switch. The shipped //jni:dds_shared already depends on both. +# //library/src:dds is listed explicitly for readability; //library/src/api:dds_c_api +# already depends on it. The test needs no static-memory initialization: +# SolverContext(SolverConfig) allocates its own ThreadData, so every entry point +# here is driven from context-owned state rather than InitializeStaticMemory(). +# A test that does need it should call InitializeStaticMemory() in a fixture, as +# library/tests/system/context_tt_facade_test.cpp does. cc_test( name = "dds_c_api_test", srcs = ["dds_c_api_test.cpp"], From ca36ba0dc293d66eac772dabee6157f6f2ea2ff1 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 20:16:42 +0100 Subject: [PATCH 16/35] Surface native failures in Release, not only in DEBUG ThrowIfError carried [Conditional("DEBUG")], which elides every call site in a Release build. In the configuration consumers actually ship, SolveBoard/CalcDdTable/CalcPar returned the raw RETURN_* code and never threw -- the opposite of what specs/dotnet-binding.md promises. Drop the attribute so the documented contract holds in every build, and add a test driving RETURN_SOLNS_WRONG_HI that fails in Release if the check is ever made conditional again. The spec bullet now says so explicitly. Note this is consumer-visible: code that previously inspected return codes in Release will now see exceptions on failure paths. Co-Authored-By: Claude Opus 4.8 --- dotnet/DDS_Core.Tests/ContextLifecycleTests.cs | 15 +++++++++++++++ dotnet/DDS_Core/DataModel/SolverContext.cs | 9 ++++++--- specs/dotnet-binding.md | 5 ++++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs index 40630f14..04774ec8 100644 --- a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs +++ b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs @@ -25,6 +25,21 @@ public void ConstructedFromConfig_SolvesReferenceDeal(TTKind kind) Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); } + /// + /// Failures must throw at the public surface in every build configuration, + /// not only in DEBUG. `solutions = 4` is out of range and the solver returns + /// RETURN_SOLNS_WRONG_HI (-9); if ThrowIfError is ever made conditional + /// again, this test fails in Release. + /// + [Fact] + public void SolveBoard_WithInvalidSolutions_Throws() + { + using var ctx = new SolverContext(); + + Assert.Throws( + () => ctx.SolveBoard(TestDeals.Reference(), -1, 4, 1, out FutureTricks _)); + } + [Fact] public void TtReconfiguration_LeavesContextUsable() { diff --git a/dotnet/DDS_Core/DataModel/SolverContext.cs b/dotnet/DDS_Core/DataModel/SolverContext.cs index cb288263..218399d8 100644 --- a/dotnet/DDS_Core/DataModel/SolverContext.cs +++ b/dotnet/DDS_Core/DataModel/SolverContext.cs @@ -1,5 +1,4 @@ -using System.Diagnostics; -using DDS_Core.Helpers; +using DDS_Core.Helpers; using DDS_Core.Native; namespace DDS_Core; @@ -141,7 +140,11 @@ public int CalcPar( in DdTableDeal table_deal #endregion #region private methods - [Conditional("DEBUG")] + // Deliberately not [Conditional("DEBUG")]: that elided every call site in + // Release, so the configuration consumers actually ship returned the raw + // RETURN_* code and never threw, contradicting the documented contract in + // specs/dotnet-binding.md. The check is one integer compare on a call that + // has just run a search. private static void ThrowIfError(int result, string functionName) { if (result != (int)SolveBoardResult.NoFault) diff --git a/specs/dotnet-binding.md b/specs/dotnet-binding.md index f5804684..e51b2006 100644 --- a/specs/dotnet-binding.md +++ b/specs/dotnet-binding.md @@ -61,7 +61,10 @@ the C++ API. context per thread, as with every binding (see [solver-context](solver-context.md)). - **Integer status returns.** Entry points return `RETURN_*` codes as elsewhere - in the API; the wrapper converts failures to exceptions at its public surface. + in the API; the wrapper converts failures to exceptions at its public surface, + in every build configuration. The check must not be made conditional on + `DEBUG` — that silently downgrades Release consumers to unchecked return + codes, which is the opposite of what this bullet promises. - **All three ABIs are covered by CI.** The managed tests run on Linux (SysV x86-64), Windows (Win64), and macOS (AArch64), each against the Bazel-built shared library located via `bazel info bazel-bin`. Windows matters most: it is From e5ab175379be7cc510acb0c6dac61d7e3dd41b55 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 20:26:34 +0100 Subject: [PATCH 17/35] Detect failed context creation via IsInvalid, not a null check The `?? throw` on both constructors was dead code. A P/Invoke returning a SafeHandle-derived type never yields null -- the marshaller constructs an instance and stores whatever pointer came back -- so when dds_c_create_solvercontext returns NULL the caller got a handle with IsInvalid == true, the throw never fired, and every subsequent call passed IntPtr.Zero into the shim to be null-guarded into RETURN_UNKNOWN_FAULT. Test the handle instead, disposing the failed one, and assert on the successful path that construction yields a live pointer. Co-Authored-By: Claude Opus 4.8 --- .../DDS_Core.Tests/ContextLifecycleTests.cs | 19 ++++++++++++ dotnet/DDS_Core/DataModel/SolverContext.cs | 29 +++++++++++++++---- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs index 04774ec8..c4dbd810 100644 --- a/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs +++ b/dotnet/DDS_Core.Tests/ContextLifecycleTests.cs @@ -25,6 +25,25 @@ public void ConstructedFromConfig_SolvesReferenceDeal(TTKind kind) Assert.Equal(TestDeals.ExpectedTricks, fut.Score[0]); } + /// + /// A successfully created context must hold a valid native handle. The + /// creation guard cannot test the reference for null — the SafeHandle + /// marshaller never returns one — so it tests IsInvalid, and this pins that + /// the successful path actually produces a live pointer rather than + /// IntPtr.Zero silently sailing through. + /// + [Theory] + [InlineData(TTKind.Small)] + [InlineData(TTKind.Large)] + public void Construction_YieldsValidHandle(TTKind kind) + { + using var fromConfig = new SolverContext(new SolverConfig(kind, 0, 0)); + Assert.False(fromConfig.Handle.IsInvalid); + + using var fromDefault = new SolverContext(); + Assert.False(fromDefault.Handle.IsInvalid); + } + /// /// Failures must throw at the public surface in every build configuration, /// not only in DEBUG. `solutions = 4` is out of range and the solver returns diff --git a/dotnet/DDS_Core/DataModel/SolverContext.cs b/dotnet/DDS_Core/DataModel/SolverContext.cs index 218399d8..87d0f81c 100644 --- a/dotnet/DDS_Core/DataModel/SolverContext.cs +++ b/dotnet/DDS_Core/DataModel/SolverContext.cs @@ -10,18 +10,35 @@ public sealed class SolverContext : IDisposable #region Constructors and destructors public SolverContext() { - Handle = DdsNative.dds_create_solvercontext_default() - ?? throw new InvalidOperationException("Failed to create SolverContext."); + Handle = Validated(DdsNative.dds_create_solvercontext_default()); } public SolverContext(SolverConfig config) { // Unpacked into scalars: the native shim is pointer-only and // POD-only, so SolverConfig never crosses the ABI boundary. - Handle = DdsNative.dds_create_solvercontext( (int) config.TTKind - , config.DefaultMemoryMB - , config.MaximumMemoryMB) - ?? throw new InvalidOperationException("Failed to create SolverContext."); + Handle = Validated(DdsNative.dds_create_solvercontext( (int) config.TTKind + , config.DefaultMemoryMB + , config.MaximumMemoryMB)); + } + + /// + /// Rejects a failed native creation. The shim returns NULL on failure, + /// but a P/Invoke returning a SafeHandle-derived type never yields null: + /// the marshaller constructs an instance and stores whatever pointer came + /// back, so failure surfaces as IsInvalid, not as a null reference. A + /// `?? throw` here would never fire, leaving every later call to pass + /// IntPtr.Zero into the shim and quietly collect RETURN_UNKNOWN_FAULT. + /// + private static SolverContextHandle Validated(SolverContextHandle handle) + { + if (handle is null || handle.IsInvalid) + { + handle?.Dispose(); + throw new InvalidOperationException("Failed to create SolverContext."); + } + + return handle; } public void Dispose() From 3cfa7e8d22b1c958b3d45c90e21d11b2ee2d5829 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Mon, 20 Jul 2026 22:36:18 +0100 Subject: [PATCH 18/35] Fix out-of-bounds aggr_target in the TransTableS reset test The lookup added to ResetAfterReturnAllMemoryIsInert used CreateTestAggrTarget, which fills aggr_target with {0x1111, 0x2222, 0x3333, 0x4444}. lookup() indexes aggp_[aggr_target[ss]], and aggp_ has 8192 (2^13) slots because aggr_target is a 13-bit rank mask -- 0x2222 = 8738 reads past the end. With an all-zero hand_dist the lookup key matches the rebuilt tree root, so it reached that indexing and crashed under a page layout where the overrun hits unmapped memory (it read adjacent heap and passed in fastbuild). Use an in-range all-zero aggr_target. Confirmed under ASan that the test is clean with the reset guard and still SEGVs without it, so it remains a genuine guard for the fix. Co-Authored-By: Claude Opus 4.8 --- library/tests/trans_table/trans_table_s_test.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/tests/trans_table/trans_table_s_test.cpp b/library/tests/trans_table/trans_table_s_test.cpp index a0239304..10f7d94e 100644 --- a/library/tests/trans_table/trans_table_s_test.cpp +++ b/library/tests/trans_table/trans_table_s_test.cpp @@ -58,8 +58,11 @@ TEST(TransTableSMemoryTest, ResetAfterReturnAllMemoryIsInert) { tt.init(handLookup); tt.reset_memory(ResetReason::NewDeal); - unsigned short aggrTarget[DDS_SUITS]; - CreateTestAggrTarget(aggrTarget); + // aggr_target entries index aggp_ (8192 = 2^13 slots), so each must be a + // 13-bit rank mask; a wider value reads past the array. An all-zero + // hand_dist matches the tree root init_tt() rebuilt, so this reaches the + // aggp_ indexing and the pos_search_point_ null check on the rebuilt table. + unsigned short aggrTarget[DDS_SUITS] = {0, 0, 0, 0}; int hand_dist[4] = {0, 0, 0, 0}; bool lowerFlag = false; From 87942623a02171ce30be2d7bfb57931298398422 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Tue, 21 Jul 2026 11:15:03 +0100 Subject: [PATCH 19/35] Run the .NET tests in Release as well as Debug in CI ThrowIfError is deliberately not [Conditional("DEBUG")], so a regression to the conditional form only changes behaviour in Release. CI built DDS_Core in Debug only, where DEBUG is defined and the call is emitted regardless -- so SolveBoard_WithInvalidSolutions_Throws, whose whole purpose is to guard that Release still throws, would still pass against a reverted [Conditional("DEBUG")]. The guarantee was untested. Add a second `dotnet test -c Release` run on all three platforms so the regression actually fails CI, and make the test's "every build configuration" claim literally true. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci_linux.yml | 11 +++++++---- .github/workflows/ci_macos.yml | 3 +++ .github/workflows/ci_windows.yml | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index df172834..95d294b0 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -80,10 +80,13 @@ jobs: # for it rather than hardcoding a path that a different --config would move. - name: Test .NET binding run: | - bazel build --verbose_failures //jni:dds_shared - export DDS_LIBRARY_PATH="$(bazel info bazel-bin)/jni/libdds.so" - test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } - dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + bazel build --verbose_failures //jni:dds_shared + export DDS_LIBRARY_PATH="$(bazel info bazel-bin)/jni/libdds.so" + test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } + # Both configurations: ThrowIfError is unconditional, so a regression to + # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. + dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal # 11 Upload test logs - name: Upload test logs - Linux diff --git a/.github/workflows/ci_macos.yml b/.github/workflows/ci_macos.yml index ca8a0447..a99090af 100644 --- a/.github/workflows/ci_macos.yml +++ b/.github/workflows/ci_macos.yml @@ -66,7 +66,10 @@ jobs: bazelisk build --verbose_failures //jni:dds_shared export DDS_LIBRARY_PATH="$(bazelisk info bazel-bin)/jni/libdds.dylib" test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } + # Both configurations: ThrowIfError is unconditional, so a regression to + # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal # Upload test logs - name: Upload test logs - macOS diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index b0a55be7..2356f670 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -80,8 +80,12 @@ jobs: Write-Host "native library not found at $env:DDS_LIBRARY_PATH" exit 1 } + # Both configurations: ThrowIfError is unconditional, so a regression to + # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal if ($LASTEXITCODE -ne 0) { exit 1 } + dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } # Upload test logs - name: Upload test logs - Windows From 2fb3e9dcd79346134e39b570a24d7eb9ce4c6261 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Tue, 21 Jul 2026 11:15:13 +0100 Subject: [PATCH 20/35] Sync clear_tt developer note with its disposal semantics The reset-semantics note still said clear_tt() "returns all TT memory to the system", while the function's @brief body and the .cpp now describe disposing the TT instance. Reword the note to match so the header does not describe two different behaviours for the same call. Co-Authored-By: Claude Opus 4.8 --- library/src/solver_context/solver_context.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/src/solver_context/solver_context.hpp b/library/src/solver_context/solver_context.hpp index 1c63b7fd..cb05081d 100644 --- a/library/src/solver_context/solver_context.hpp +++ b/library/src/solver_context/solver_context.hpp @@ -175,8 +175,8 @@ class SolverContext // tt->reset_memory(FreeMemory) when a TT exists; // preserves the TT allocation for reuse. // reset_best_moves_lite() — clears only best-move ranks and updates memUsed. - // clear_tt() — returns all TT memory to the system; preserves - // future config and recreates lazily on demand. + // clear_tt() — disposes the TT instance; preserves future + // config and recreates lazily on demand. // dispose_trans_table() — destroys the owned TT immediately. // - Diagnostics: When built with DDS_UTILITIES_LOG / DDS_UTILITIES_STATS, TT // lifecycle events append compact log entries and bump small counters. From 8e994ec10faa089524a692808217377c1f5fff7d Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Fri, 14 Aug 2026 09:03:35 +0100 Subject: [PATCH 21/35] fixes a syntax error in linux spec. --- .github/workflows/ci_linux.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index 95d294b0..5d78b1fd 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -74,7 +74,8 @@ jobs: # runtime exists, which is not the case here. - name: Setup .NET uses: actions/setup-dotnet@v4 - with: dotnet-version: "8.0.x" + with: + dotnet-version: "8.0.x" # bazel-bin is a configuration-dependent convenience symlink, so ask Bazel # for it rather than hardcoding a path that a different --config would move. From 4cc77548d3a449785c17d6ca503c68d00d0be802 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Fri, 14 Aug 2026 09:26:01 +0100 Subject: [PATCH 22/35] updates from calling bazel directly to using bazelisk. --- .github/workflows/ci_linux.yml | 4 ++-- .github/workflows/ci_windows.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index 5d78b1fd..54008e7b 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -81,8 +81,8 @@ jobs: # for it rather than hardcoding a path that a different --config would move. - name: Test .NET binding run: | - bazel build --verbose_failures //jni:dds_shared - export DDS_LIBRARY_PATH="$(bazel info bazel-bin)/jni/libdds.so" + bazelisk build --verbose_failures //jni:dds_shared + export DDS_LIBRARY_PATH="$(bazelisk info bazel-bin)/jni/libdds.so" test -f "$DDS_LIBRARY_PATH" || { echo "native library not found at $DDS_LIBRARY_PATH"; exit 1; } # Both configurations: ThrowIfError is unconditional, so a regression to # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index 2356f670..a3a1af2e 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -72,9 +72,9 @@ jobs: - name: Test .NET binding shell: pwsh run: | - bazel build --verbose_failures //jni:dds_shared + bazelisk build --verbose_failures //jni:dds_shared if ($LASTEXITCODE -ne 0) { exit 1 } - $binDir = bazel info bazel-bin + $binDir = bazelisk info bazel-bin $env:DDS_LIBRARY_PATH = Join-Path $binDir "jni\dds.dll" if (-not (Test-Path $env:DDS_LIBRARY_PATH)) { Write-Host "native library not found at $env:DDS_LIBRARY_PATH" From b59967facc998578cedf38b86ce7a5e5bcbfef07 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 14 Aug 2026 21:27:42 +0200 Subject: [PATCH 23/35] Add .NET dd_table_for_deal CLI matching the C++/Python tools. Provides a managed counterpart that solves PBN deals via DDS_Core, with unit-tested helpers and CI coverage. Co-authored-by: Cursor --- .github/workflows/ci_linux.yml | 3 + .github/workflows/ci_macos.yml | 3 + .github/workflows/ci_windows.yml | 5 + docs/dotnet_interface.md | 12 + .../DdTableForDeal.Tests.csproj | 26 ++ .../DdTableForDealLibTests.cs | 312 +++++++++++++++ dotnet/DdTableForDeal/DdTableForDeal.csproj | 19 + dotnet/DdTableForDeal/DdTableForDealLib.cs | 365 ++++++++++++++++++ dotnet/DdTableForDeal/Program.cs | 229 +++++++++++ examples/README | 10 +- 10 files changed, 983 insertions(+), 1 deletion(-) create mode 100644 dotnet/DdTableForDeal.Tests/DdTableForDeal.Tests.csproj create mode 100644 dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs create mode 100644 dotnet/DdTableForDeal/DdTableForDeal.csproj create mode 100644 dotnet/DdTableForDeal/DdTableForDealLib.cs create mode 100644 dotnet/DdTableForDeal/Program.cs diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index 54008e7b..9e3ee8f7 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -88,6 +88,9 @@ jobs: # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal + # Pure managed CLI helpers (no native library required). + dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal + dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal # 11 Upload test logs - name: Upload test logs - Linux diff --git a/.github/workflows/ci_macos.yml b/.github/workflows/ci_macos.yml index a99090af..ab6109dd 100644 --- a/.github/workflows/ci_macos.yml +++ b/.github/workflows/ci_macos.yml @@ -70,6 +70,9 @@ jobs: # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal + # Pure managed CLI helpers (no native library required). + dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal + dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal # Upload test logs - name: Upload test logs - macOS diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index a3a1af2e..db9ac3c3 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -86,6 +86,11 @@ jobs: if ($LASTEXITCODE -ne 0) { exit 1 } dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal if ($LASTEXITCODE -ne 0) { exit 1 } + # Pure managed CLI helpers (no native library required). + dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } # Upload test logs - name: Upload test logs - Windows diff --git a/docs/dotnet_interface.md b/docs/dotnet_interface.md index 75d24a4a..5c7027d8 100644 --- a/docs/dotnet_interface.md +++ b/docs/dotnet_interface.md @@ -88,6 +88,18 @@ three artifacts above. There are two ways to point it at a library: If the variable is set but the library cannot be loaded, the error names the path you gave rather than reporting a missing entry point later. +## Example: `dd_table_for_deal` + +`dotnet/DdTableForDeal/` is the .NET counterpart of `examples/dd_table_for_deal` +and `python/examples/dd_table_for_deal.py`. After building the native library and +setting `DDS_LIBRARY_PATH` as above: + +```bash +dotnet run --project dotnet/DdTableForDeal/ -- hands/example.pbn +dotnet run --project dotnet/DdTableForDeal/ -- --vul ns hands/example.pbn +dotnet test dotnet/DdTableForDeal.Tests/ +``` + ## Which native symbols are used The modern context API binds the pure-C shim (`dds_c_*`, from diff --git a/dotnet/DdTableForDeal.Tests/DdTableForDeal.Tests.csproj b/dotnet/DdTableForDeal.Tests/DdTableForDeal.Tests.csproj new file mode 100644 index 00000000..809e8815 --- /dev/null +++ b/dotnet/DdTableForDeal.Tests/DdTableForDeal.Tests.csproj @@ -0,0 +1,26 @@ + + + + net8.0 + enable + enable + false + AnyCPU + Major + + + + + + + + + + + + + + + + + diff --git a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs new file mode 100644 index 00000000..30dc91ae --- /dev/null +++ b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs @@ -0,0 +1,312 @@ +using DDS_Core; + +namespace DdTableForDeal.Tests; + +public class ParseVulnerableTests +{ + [Theory] + [InlineData("none", 0)] + [InlineData("None", 0)] + [InlineData("0", 0)] + [InlineData("both", 1)] + [InlineData("1", 1)] + [InlineData("ns", 2)] + [InlineData("NS", 2)] + [InlineData("2", 2)] + [InlineData("ew", 3)] + [InlineData("3", 3)] + public void AcceptsAliasesAndCodes(string text, int expected) + { + Assert.Equal(expected, DdTableForDealLib.ParseVulnerable(text)); + } + + [Theory] + [InlineData("")] + [InlineData("maybe")] + [InlineData("4")] + public void RejectsUnknown(string text) + { + Assert.Null(DdTableForDealLib.ParseVulnerable(text)); + } +} + +public class ParseLimitTests +{ + [Theory] + [InlineData("1", 1u)] + [InlineData("25", 25u)] + public void AcceptsPositiveIntegers(string text, uint expected) + { + Assert.Equal(expected, DdTableForDealLib.ParseLimit(text)); + } + + [Theory] + [InlineData("")] + [InlineData("0")] + [InlineData("-1")] + [InlineData("3x")] + [InlineData("1.5")] + public void RejectsNonPositiveAndNonNumeric(string text) + { + Assert.Null(DdTableForDealLib.ParseLimit(text)); + } +} + +public class ApplyDealLimitTests +{ + [Fact] + public void KeepsPrefixWhenLimited() + { + string[] deals = ["a", "b", "c"]; + Assert.Equal(["a", "b"], DdTableForDealLib.ApplyDealLimit(deals, 2)); + Assert.Equal(deals, DdTableForDealLib.ApplyDealLimit(deals, null)); + Assert.Equal(deals, DdTableForDealLib.ApplyDealLimit(deals, 10)); + } +} + +public class ExtractDealTagsTests +{ + [Fact] + public void FindsAllTags() + { + const string text = + "{Board 1}\n" + + "[Deal \"N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93\"]\n" + + "\n" + + "{Board 2}\n" + + "[Deal \"N:QJ6.K652.J85.T98 873.J97.AT764.Q4 " + + "K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3\"]\n"; + + var deals = DdTableForDealLib.ExtractDealTags(text); + Assert.Equal(2, deals.Count); + Assert.Equal( + "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93", + deals[0]); + Assert.Equal( + "N:QJ6.K652.J85.T98 873.J97.AT764.Q4 " + + "K5.T83.KQ9.A7652 AT942.AQ4.32.KJ3", + deals[1]); + } + + [Fact] + public void EmptyWhenNoTags() + { + Assert.Empty(DdTableForDealLib.ExtractDealTags("{comment only}")); + } +} + +public class UniqueDealsTests +{ + [Fact] + public void PreservesFirstSeenOrderAndDropsDuplicates() + { + string[] deals = ["deal-a", "deal-b", "deal-a", "deal-c", "deal-b", "deal-a"]; + var unique = DdTableForDealLib.UniqueDeals(deals); + Assert.Equal(["deal-a", "deal-b", "deal-c"], unique); + } +} + +public class LooksLikePathTests +{ + [Fact] + public void DetectsPathsAndExtensions() + { + Assert.True(DdTableForDealLib.LooksLikePath("boards.pbn")); + Assert.True(DdTableForDealLib.LooksLikePath("hands/x.pbn")); + Assert.True(DdTableForDealLib.LooksLikePath("notes.txt")); + Assert.False(DdTableForDealLib.LooksLikePath( + "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93")); + } +} + +public class ParseCliTests +{ + private const string ExampleDeal = + "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93"; + + [Fact] + public void DealOnlyDefaultsVulnerableToNone() + { + var parsed = DdTableForDealLib.ParseCli(["prog", ExampleDeal]); + Assert.NotNull(parsed); + Assert.Equal(ExampleDeal, parsed.Value.DealArg); + Assert.Equal(0, parsed.Value.Vulnerable); + Assert.Null(parsed.Value.Limit); + } + + [Fact] + public void VulFlagBeforeDeal() + { + var parsed = DdTableForDealLib.ParseCli(["prog", "--vul", "ns", ExampleDeal]); + Assert.NotNull(parsed); + Assert.Equal(ExampleDeal, parsed.Value.DealArg); + Assert.Equal(2, parsed.Value.Vulnerable); + Assert.Null(parsed.Value.Limit); + } + + [Fact] + public void LimitAndVulTogether() + { + var parsed = DdTableForDealLib.ParseCli( + ["prog", "--vul", "ns", "--limit", "1", "boards.pbn"]); + Assert.NotNull(parsed); + Assert.Equal("boards.pbn", parsed.Value.DealArg); + Assert.Equal(2, parsed.Value.Vulnerable); + Assert.Equal(1u, parsed.Value.Limit); + } + + [Fact] + public void HelpReturnsNull() + { + Assert.Null(DdTableForDealLib.ParseCli(["prog", "--help"])); + Assert.Null(DdTableForDealLib.ParseCli(["prog", "-h"])); + } + + [Fact] + public void UnknownOptionThrows() + { + Assert.Throws(() => + DdTableForDealLib.ParseCli(["prog", "--nope", ExampleDeal])); + } +} + +public class FormatParLineTests +{ + private static ContractType MakeContract( + int seats, int level, int denom, int underTricks, int overTricks) => + new() + { + Seats = seats, + Level = level, + Denomination = denom, + UnderTricks = underTricks, + OverTricks = overTricks, + }; + + [Fact] + public void SingleSacrifice() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = -300; + sides[0].Number = 1; + sides[0].Contracts[0] = MakeContract(/*NS*/ 4, 5, /*H*/ 2, 2, 0); + sides[1].Score = 300; + sides[1].Number = 1; + sides[1].Contracts[0] = MakeContract(/*NS*/ 4, 5, /*H*/ 2, 2, 0); + + Assert.Equal("Par: NS 5Hx -2 -300", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void SingleMakingUsesEqualsAndDeclaringScore() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = -110; + sides[0].Number = 1; + sides[0].Contracts[0] = MakeContract(/*EW*/ 5, 2, /*S*/ 1, 0, 0); + sides[1].Score = 110; + sides[1].Number = 1; + sides[1].Contracts[0] = MakeContract(/*EW*/ 5, 2, /*S*/ 1, 0, 0); + + Assert.Equal("Par: EW 2S = 110", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void MultipleSacrificesOnOneLine() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = 100; + sides[0].Number = 2; + sides[0].Contracts[0] = MakeContract(/*EW*/ 5, 3, /*D*/ 3, 1, 0); + sides[0].Contracts[1] = MakeContract(/*EW*/ 5, 3, /*C*/ 4, 1, 0); + sides[1].Score = -100; + sides[1].Number = 2; + sides[1].Contracts[0] = MakeContract(/*EW*/ 5, 3, /*D*/ 3, 1, 0); + sides[1].Contracts[1] = MakeContract(/*EW*/ 5, 3, /*C*/ 4, 1, 0); + + Assert.Equal("Par: EW 3Dx, 3Cx -1 -100", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void OmitsRepeatedDeclaringSideWhenSeatsDiffer() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = 100; + sides[0].Number = 2; + sides[0].Contracts[0] = MakeContract(/*EW*/ 5, 4, /*H*/ 2, 1, 0); + sides[0].Contracts[1] = MakeContract(/*E*/ 1, 5, /*C*/ 4, 1, 0); + sides[1].Score = -100; + sides[1].Number = 2; + sides[1].Contracts[0] = MakeContract(/*EW*/ 5, 4, /*H*/ 2, 1, 0); + sides[1].Contracts[1] = MakeContract(/*E*/ 1, 5, /*C*/ 4, 1, 0); + + Assert.Equal("Par: EW 4Hx, 5Cx -1 -100", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void PassedOut() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = 0; + sides[0].Number = 1; + sides[1].Score = 0; + sides[1].Number = 1; + + Assert.Equal("Par: 0", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void ReturnsNullWhenNoContractsDespiteScores() + { + var sides = new ParResultsMaster[2]; + sides[0].Score = -100; + sides[0].Number = 0; + sides[1].Score = 100; + sides[1].Number = 0; + + Assert.Null(DdTableForDealLib.FormatParLine(sides)); + } +} + +public class FormatTableTests +{ + [Fact] + public void MatchesCppColumnOrder() + { + // strain rows: NT=4, then S/H/D/C = 0..3; columns North/South/East/West = 0,2,1,3 + var table = new DdTableResults(); + for (int strain = 0; strain < 5; strain++) + for (int hand = 0; hand < 4; hand++) + table.ResultsTable[strain, hand] = strain * 10 + hand; + + var text = DdTableForDealLib.FormatTable(table); + var lines = text.Split('\n', StringSplitOptions.RemoveEmptyEntries); + + Assert.Equal(6, lines.Length); + Assert.Equal(" North South East West ", lines[0]); + Assert.Equal(" NT 40 42 41 43", lines[1]); + Assert.Equal(" S 0 2 1 3", lines[2]); + Assert.Equal(" H 10 12 11 13", lines[3]); + Assert.Equal(" D 20 22 21 23", lines[4]); + Assert.Equal(" C 30 32 31 33", lines[5]); + } +} + +public class FormatPbnHandTests +{ + [Fact] + public void EndsWithBlankLineAfterDiagram() + { + const string deal = + "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93"; + + string text = DdTableForDealLib.FormatPbnHand("dd_table_for_deal:\n", deal); + + Assert.EndsWith("\n\n", text); + } +} diff --git a/dotnet/DdTableForDeal/DdTableForDeal.csproj b/dotnet/DdTableForDeal/DdTableForDeal.csproj new file mode 100644 index 00000000..99b57d6a --- /dev/null +++ b/dotnet/DdTableForDeal/DdTableForDeal.csproj @@ -0,0 +1,19 @@ + + + + Exe + net8.0 + enable + enable + AnyCPU + false + ..\..\Build\bin\ + DdTableForDeal + dd_table_for_deal + + + + + + + diff --git a/dotnet/DdTableForDeal/DdTableForDealLib.cs b/dotnet/DdTableForDeal/DdTableForDealLib.cs new file mode 100644 index 00000000..50347030 --- /dev/null +++ b/dotnet/DdTableForDeal/DdTableForDealLib.cs @@ -0,0 +1,365 @@ +using System.Text; +using System.Text.RegularExpressions; +using DDS_Core; + +namespace DdTableForDeal; + +/// +/// Pure helpers for the .NET dd_table_for_deal CLI — counterpart to +/// examples/dd_table_for_deal_lib.cpp / python/examples/dd_table_for_deal.py. +/// +public static partial class DdTableForDealLib +{ + public const int PbnFileMax = 16 * 1024 * 1024; + public const int PbnDealMax = 80; + + // Matches dll.h contractType.denom: 0=NT, 1=S, 2=H, 3=D, 4=C. + private static readonly char[] DenomChars = ['N', 'S', 'H', 'D', 'C']; + private static readonly string[] SeatNames = ["N", "E", "S", "W", "NS", "EW"]; + + private static readonly (string Label, int Strain)[] StrainRows = + [ + ("NT", 4), + ("S", 0), + ("H", 1), + ("D", 2), + ("C", 3), + ]; + + // North, South, East, West — matches examples/hands.cpp print_table. + private static readonly int[] HandColumns = [0, 2, 1, 3]; + + private static readonly int[] BitMapRank = + [ + 0x0000, 0x0000, 0x0001, 0x0002, 0x0004, 0x0008, 0x0010, 0x0020, + 0x0040, 0x0080, 0x0100, 0x0200, 0x0400, 0x0800, 0x1000, 0x2000, + ]; + + private const string CardRankChars = "xx23456789TJQKA-"; + private const int DdsFullLine = 80; + private const int DdsHandOffset = 12; + private const int DdsHandLines = 12; + + public readonly record struct CliOptions(string DealArg, int Vulnerable, uint? Limit); + + [GeneratedRegex(@"\[Deal\s*""([^""]*)""", RegexOptions.IgnoreCase)] + private static partial Regex DealTagRegex(); + + public static int? ParseVulnerable(string text) + { + return text.ToLowerInvariant() switch + { + "none" or "0" => 0, + "both" or "1" => 1, + "ns" or "2" => 2, + "ew" or "3" => 3, + _ => null, + }; + } + + public static uint? ParseLimit(string text) + { + if (string.IsNullOrEmpty(text)) + return null; + + uint value = 0; + foreach (char ch in text) + { + if (ch is < '0' or > '9') + return null; + uint digit = (uint)(ch - '0'); + if (value > (uint.MaxValue - digit) / 10) + return null; + value = value * 10 + digit; + } + + return value == 0 ? null : value; + } + + public static IReadOnlyList ApplyDealLimit( + IReadOnlyList deals, uint? limit) + { + if (limit is null || limit.Value >= deals.Count) + return deals is List list ? list : deals.ToList(); + return deals.Take((int)limit.Value).ToList(); + } + + public static IReadOnlyList ExtractDealTags(string text) + { + var deals = new List(); + foreach (Match match in DealTagRegex().Matches(text)) + deals.Add(match.Groups[1].Value); + return deals; + } + + public static IReadOnlyList UniqueDeals(IEnumerable deals) + { + var unique = new List(); + var seen = new HashSet(); + foreach (string deal in deals) + { + if (seen.Add(deal)) + unique.Add(deal); + } + return unique; + } + + public static bool LooksLikePath(string arg) + { + if (arg.Contains('/') || arg.Contains('\\')) + return true; + return arg.EndsWith(".pbn", StringComparison.OrdinalIgnoreCase) + || arg.EndsWith(".txt", StringComparison.OrdinalIgnoreCase); + } + + /// + /// Parse CLI args (argv[0] is the program name). Returns null for help. + /// When is false and no deal arg is given, uses "-". + /// + public static CliOptions? ParseCli(IReadOnlyList argv, bool stdinIsTty = true) + { + string? deal = null; + int vulnerable = 0; + uint? limit = null; + + for (int i = 1; i < argv.Count; i++) + { + string arg = argv[i]; + if (arg is "-h" or "--help") + return null; + + if (arg == "--vul") + { + if (i + 1 >= argv.Count) + throw new ArgumentException( + "--vul requires a value (none|both|ns|ew or 0|1|2|3)"); + vulnerable = ParseVulnerable(argv[++i]) + ?? throw new ArgumentException( + "Invalid --vul value (use none|both|ns|ew or 0|1|2|3)"); + continue; + } + + if (arg == "--limit") + { + if (i + 1 >= argv.Count) + throw new ArgumentException("--limit requires a positive integer"); + limit = ParseLimit(argv[++i]) + ?? throw new ArgumentException( + "Invalid --limit value (use a positive integer)"); + continue; + } + + if (arg.StartsWith('-') && arg != "-") + throw new ArgumentException($"Unknown option: {arg}"); + + if (deal is not null) + throw new ArgumentException("Only one deal argument is allowed"); + + deal = arg; + } + + if (deal is null) + { + if (!stdinIsTty) + deal = "-"; + else + throw new ArgumentException("missing deal argument"); + } + + return new CliOptions(deal, vulnerable, limit); + } + + public static string? FormatParLine(ReadOnlySpan sides) + { + if (sides.Length < 2) + return null; + + if (sides[0].Score == 0 && sides[1].Score == 0) + return "Par: 0"; + + if (sides[0].Number <= 0 && sides[1].Number <= 0) + return null; + + ContractType first = sides[0].Number > 0 + ? sides[0].Contracts[0] + : sides[1].Contracts[0]; + int side = first.Seats is 4 or 0 or 2 ? 0 : 1; + ParResultsMaster chosen = sides[side]; + if (chosen.Number <= 0) + return null; + + var body = new StringBuilder(); + for (int i = 0; i < chosen.Number; i++) + { + string? piece = FormatContract(chosen.Contracts[i], includeSeats: i == 0); + if (piece is null) + return null; + if (i > 0) + body.Append(", "); + body.Append(piece); + } + + ContractType contract = chosen.Contracts[0]; + string result = contract.UnderTricks > 0 + ? $"-{contract.UnderTricks}" + : contract.OverTricks > 0 + ? $"+{contract.OverTricks}" + : "="; + + return $"Par: {body} {result} {chosen.Score}"; + } + + public static string FormatTable(in DdTableResults table) + { + var sb = new StringBuilder(); + sb.AppendFormat("{0,5} {1,-5} {2,-5} {3,-5} {4,-5}\n", + "", "North", "South", "East", "West"); + + foreach ((string label, int strain) in StrainRows) + { + sb.AppendFormat("{0,5} {1,5} {2,5} {3,5} {4,5}\n", + label, + table.ResultsTable[strain, HandColumns[0]], + table.ResultsTable[strain, HandColumns[1]], + table.ResultsTable[strain, HandColumns[2]], + table.ResultsTable[strain, HandColumns[3]]); + } + + return sb.ToString().TrimEnd('\n'); + } + + public static string FormatPbnHand(string title, string pbnDeal) + { + uint[,] remainCards = ConvertPbn(pbnDeal); + char[][] text = new char[DdsHandLines][]; + int[] rowEnds = new int[DdsHandLines]; + for (int i = 0; i < DdsHandLines; i++) + { + text[i] = new char[DdsFullLine]; + Array.Fill(text[i], ' '); + rowEnds[i] = DdsFullLine; + } + + for (int h = 0; h < 4; h++) + { + int offset, line; + switch (h) + { + case 0: offset = DdsHandOffset; line = 0; break; + case 1: offset = 2 * DdsHandOffset; line = 4; break; + case 2: offset = DdsHandOffset; line = 8; break; + default: offset = 0; line = 4; break; + } + + for (int s = 0; s < 4; s++) + { + int row = line + s; + int c = offset; + for (int r = 14; r >= 2; r--) + { + if (((remainCards[h, s] >> 2) & BitMapRank[r]) != 0) + text[row][c++] = CardRankChars[r]; + } + if (c == offset) + text[row][c++] = '-'; + if (h != 3) + rowEnds[row] = c; + } + } + + var sb = new StringBuilder(); + sb.Append(title); + int dashLen = Math.Max(0, title.Length - 1); + sb.Append('-', dashLen).Append('\n'); + for (int i = 0; i < DdsHandLines; i++) + sb.Append(text[i], 0, rowEnds[i]).Append('\n'); + sb.Append('\n'); // blank line after the diagram (matches C++/Python) + return sb.ToString(); + } + + public static string FormatParVerbose(in ParResults par) + { + return + $"NS score: {par.ParScores[0]}\n" + + $"EW score: {par.ParScores[1]}\n" + + $"NS list : {par.ParContractStrings[0]}\n" + + $"EW list : {par.ParContractStrings[1]}\n"; + } + + public static uint[,] ConvertPbn(string pbnDeal) + { + var remain = new uint[4, 4]; + int bp = 0; + while (bp < 3 && bp < pbnDeal.Length + && pbnDeal[bp] is not ('N' or 'W' or 'E' or 'S' + or 'n' or 'w' or 'e' or 's')) + bp++; + if (bp >= 3 || bp >= pbnDeal.Length) + return remain; + + int first = char.ToUpperInvariant(pbnDeal[bp]) switch + { + 'N' => 0, + 'E' => 1, + 'S' => 2, + _ => 3, + }; + bp += 2; + int handRelFirst = 0; + int suitInHand = 0; + + while (bp < 80 && bp < pbnDeal.Length) + { + char ch = pbnDeal[bp]; + int card = IsCard(ch); + if (card != 0) + { + int hand = first switch + { + 0 => handRelFirst, + 1 => handRelFirst == 0 ? 1 : handRelFirst == 3 ? 0 : handRelFirst + 1, + 2 => handRelFirst == 0 ? 2 : handRelFirst == 1 ? 3 : handRelFirst - 2, + _ => handRelFirst == 0 ? 3 : handRelFirst - 1, + }; + remain[hand, suitInHand] |= (uint)(BitMapRank[card] << 2); + } + else if (ch == '.') + { + suitInHand++; + } + else if (ch == ' ') + { + handRelFirst++; + suitInHand = 0; + } + bp++; + } + return remain; + } + + private static int IsCard(char ch) + { + return char.ToUpperInvariant(ch) switch + { + '2' => 2, '3' => 3, '4' => 4, '5' => 5, '6' => 6, + '7' => 7, '8' => 8, '9' => 9, 'T' => 10, 'J' => 11, + 'Q' => 12, 'K' => 13, 'A' => 14, + _ => 0, + }; + } + + private static string? FormatContract(in ContractType contract, bool includeSeats) + { + if (contract.Denomination is < 0 or > 4) + return null; + if (contract.Seats is < 0 or > 5) + return null; + + char denom = DenomChars[contract.Denomination]; + bool doubled = contract.UnderTricks > 0; + string body = doubled + ? $"{contract.Level}{denom}x" + : $"{contract.Level}{denom}"; + return includeSeats ? $"{SeatNames[contract.Seats]} {body}" : body; + } +} diff --git a/dotnet/DdTableForDeal/Program.cs b/dotnet/DdTableForDeal/Program.cs new file mode 100644 index 00000000..293c6fce --- /dev/null +++ b/dotnet/DdTableForDeal/Program.cs @@ -0,0 +1,229 @@ +using System.Text; +using DDS_Core; + +namespace DdTableForDeal; + +internal static class Program +{ + private static int Main(string[] args) + { + // ParseCli expects argv[0] = program name. + var argv = new string[args.Length + 1]; + argv[0] = Environment.GetCommandLineArgs()[0]; + args.CopyTo(argv, 1); + + DdTableForDealLib.CliOptions? parsed; + try + { + parsed = DdTableForDealLib.ParseCli(argv, stdinIsTty: !Console.IsInputRedirected); + } + catch (ArgumentException ex) + { + Console.Error.WriteLine(ex.Message); + PrintUsage(Path.GetFileName(argv[0])); + return 1; + } + + if (parsed is null) + { + PrintUsage(Path.GetFileName(argv[0])); + return 0; + } + + IReadOnlyList deals; + try + { + deals = DdTableForDealLib.UniqueDeals(LoadDeals(parsed.Value.DealArg)); + } + catch (Exception ex) when (ex is ArgumentException or IOException or InvalidOperationException) + { + Console.Error.WriteLine(ex.Message); + return 1; + } + + deals = DdTableForDealLib.ApplyDealLimit(deals, parsed.Value.Limit); + + if (deals.Any(d => d.Length >= DdTableForDealLib.PbnDealMax)) + { + Console.Error.WriteLine( + $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); + return 1; + } + + try + { + using var ctx = new SolverContext(); + var dds = new DDS(); + for (int i = 0; i < deals.Count; i++) + { + if (!ProcessDeal(ctx, dds, deals[i], i + 1, deals.Count, parsed.Value.Vulnerable)) + return 1; + } + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException + or BadImageFormatException) + { + Console.Error.WriteLine( + "Failed to load native DDS library. Build //jni:dds_shared and set " + + "DDS_LIBRARY_PATH to the full path of libdds.dylib / libdds.so / dds.dll."); + Console.Error.WriteLine(ex.Message); + return 1; + } + + return 0; + } + + private static bool ProcessDeal( + SolverContext ctx, + DDS dds, + string deal, + int dealNo, + int dealCount, + int vulnerable) + { + var tableDeal = new DdTableDealPBN { Cards = deal }; + + try + { + ctx.CalcDdTable(tableDeal, out DdTableResults table); + dds.ParAll(in table, vulnerable, out ParResultsMasters sidesMasters); + + string title = dealCount == 1 + ? "dd_table_for_deal:\n" + : $"Deal {dealNo}:\n"; + + Console.Write(DdTableForDealLib.FormatPbnHand(title, deal)); + Console.WriteLine(DdTableForDealLib.FormatTable(table)); + Console.WriteLine(); + + var sides = new ParResultsMaster[] { sidesMasters[0], sidesMasters[1] }; + string? parLine = DdTableForDealLib.FormatParLine(sides); + if (parLine is not null) + { + Console.WriteLine(parLine); + } + else + { + dds.Par(in table, vulnerable, out ParResults par); + Console.Write(DdTableForDealLib.FormatParVerbose(par)); + } + + if (dealCount > 1) + Console.WriteLine(); + return true; + } + catch (Exception ex) + { + Console.Error.WriteLine($"DDS error: {ex.Message}"); + return false; + } + } + + private static IReadOnlyList LoadDeals(string arg) + { + if (arg == "-") + { + string? text = ReadPbnStream(Console.OpenStandardInput()); + if (text is null) + throw new InvalidOperationException("Cannot read PBN from stdin"); + var deals = DdTableForDealLib.ExtractDealTags(text); + if (deals.Count == 0) + throw new InvalidOperationException("No [Deal \"...\"] tag found in stdin"); + return deals; + } + + string? fileText = ReadPbnFileWorkspaceRelative(arg); + if (fileText is not null) + { + var deals = DdTableForDealLib.ExtractDealTags(fileText); + if (deals.Count == 0) + throw new InvalidOperationException($"No [Deal \"...\"] tag found in {arg}"); + return deals; + } + + if (DdTableForDealLib.LooksLikePath(arg)) + throw new FileNotFoundException($"Cannot read file: {arg}", arg); + + if (arg.Length >= DdTableForDealLib.PbnDealMax) + throw new ArgumentException( + $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); + + return [arg]; + } + + private static string? ReadPbnFileWorkspaceRelative(string path) + { + if (TryReadPbnFile(path, out string? text)) + return text; + + string? workspace = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); + if (workspace is not null + && TryReadPbnFile(Path.Combine(workspace, path), out text)) + { + return text; + } + + return null; + } + + private static bool TryReadPbnFile(string path, out string? text) + { + text = null; + try + { + using var stream = File.OpenRead(path); + text = ReadPbnStream(stream); + return text is not null; + } + catch (IOException) + { + return false; + } + } + + private static string? ReadPbnStream(Stream stream) + { + using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, leaveOpen: true); + var sb = new StringBuilder(); + char[] buffer = new char[4096]; + while (true) + { + int n = reader.Read(buffer, 0, buffer.Length); + if (n <= 0) + break; + sb.Append(buffer, 0, n); + if (sb.Length > DdTableForDealLib.PbnFileMax) + { + Console.Error.WriteLine( + $"PBN input too large (max {DdTableForDealLib.PbnFileMax} characters)"); + return null; + } + } + return sb.ToString(); + } + + private static void PrintUsage(string prog) + { + Console.Error.Write( + $"Usage: {prog} [--vul none|both|ns|ew|0|1|2|3] [--limit N] " + + $"\n" + + $" {prog} -h | --help\n" + + "\n" + + "Calculate double-dummy tricks and par for all strains and leads.\n" + + "\n" + + "Arguments:\n" + + " DDS PBN deal string, or path to a .pbn file\n" + + " --vul Vulnerability: none|both|ns|ew or 0|1|2|3" + + " (default: none)\n" + + " --limit Solve only the first N unique deals\n" + + "\n" + + "If stdin is not a terminal, PBN is read from stdin (all [Deal \"...\"] tags).\n" + + "\n" + + "Examples:\n" + + $" {prog} \"N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93\"\n" + + $" {prog} --vul ns hands/example.pbn\n" + + $" {prog} --limit 3 hands/multi_board.pbn\n" + + $" {prog} < hands/example.pbn\n"); + } +} diff --git a/examples/README b/examples/README index 9be95858..b27e57ee 100644 --- a/examples/README +++ b/examples/README @@ -16,11 +16,19 @@ Python `dd_table_for_deal` (see `python/examples/`): bazelisk run //python/examples:dd_table_for_deal -- --vul ns hands/example.pbn bazelisk run //python/examples:dd_table_for_deal -- --limit 3 hands/multi_board.pbn +.NET `dd_table_for_deal` (see `dotnet/DdTableForDeal/`): + bazelisk build //jni:dds_shared + export DDS_LIBRARY_PATH="$(bazelisk info bazel-bin)/jni/libdds.dylib" # .so on Linux, dds.dll on Windows + dotnet run --project dotnet/DdTableForDeal/ -- hands/example.pbn + dotnet run --project dotnet/DdTableForDeal/ -- --vul ns hands/example.pbn + dotnet run --project dotnet/DdTableForDeal/ -- --limit 3 hands/multi_board.pbn + dotnet test dotnet/DdTableForDeal.Tests/ + Available examples: - analyse_all_plays_bin, analyse_all_plays_pbn - analyse_play_bin, analyse_play_pbn - calc_all_tables, calc_all_tables_pbn - calc_dd_table, calc_dd_table_pbn -- dd_table_for_deal (C++; Python: //python/examples:dd_table_for_deal) +- dd_table_for_deal (C++; Python: //python/examples:dd_table_for_deal; .NET: dotnet/DdTableForDeal) - dealer_par, par - solve_all_boards, solve_board, solve_board_pbn From acbc5135fe2a89be8a032260abd1345ff3d43108 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 14 Aug 2026 21:52:19 +0200 Subject: [PATCH 24/35] Add shell e2e for the .NET dd_table_for_deal CLI. Drive `dotnet run` from bash/PowerShell and run those scripts in Linux, macOS, and Windows CI. Co-authored-by: Cursor --- .github/workflows/ci_linux.yml | 5 +- .github/workflows/ci_macos.yml | 5 +- .github/workflows/ci_windows.yml | 5 +- docs/dotnet_interface.md | 1 + dotnet/DdTableForDeal/DdTableForDealApp.cs | 241 +++++++++++++++++++++ dotnet/DdTableForDeal/Program.cs | 223 +------------------ dotnet/DdTableForDeal/e2e.ps1 | 91 ++++++++ dotnet/DdTableForDeal/e2e.sh | 99 +++++++++ examples/README | 1 + 9 files changed, 451 insertions(+), 220 deletions(-) create mode 100644 dotnet/DdTableForDeal/DdTableForDealApp.cs create mode 100644 dotnet/DdTableForDeal/e2e.ps1 create mode 100755 dotnet/DdTableForDeal/e2e.sh diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index 9e3ee8f7..a5b62c7a 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -88,9 +88,12 @@ jobs: # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal - # Pure managed CLI helpers (no native library required). + # DdTableForDeal unit helpers (no native library required). dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal + # CLI e2e: bash drives `dotnet run` against the native library. + chmod +x dotnet/DdTableForDeal/e2e.sh + ./dotnet/DdTableForDeal/e2e.sh # 11 Upload test logs - name: Upload test logs - Linux diff --git a/.github/workflows/ci_macos.yml b/.github/workflows/ci_macos.yml index ab6109dd..71d00cde 100644 --- a/.github/workflows/ci_macos.yml +++ b/.github/workflows/ci_macos.yml @@ -70,9 +70,12 @@ jobs: # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. dotnet test dotnet/DDS_Core.Tests/ --verbosity normal dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal - # Pure managed CLI helpers (no native library required). + # DdTableForDeal unit helpers (no native library required). dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal + # CLI e2e: bash drives `dotnet run` against the native library. + chmod +x dotnet/DdTableForDeal/e2e.sh + ./dotnet/DdTableForDeal/e2e.sh # Upload test logs - name: Upload test logs - macOS diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index db9ac3c3..cc64dd0c 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -86,11 +86,14 @@ jobs: if ($LASTEXITCODE -ne 0) { exit 1 } dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal if ($LASTEXITCODE -ne 0) { exit 1 } - # Pure managed CLI helpers (no native library required). + # DdTableForDeal unit helpers (no native library required). dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal if ($LASTEXITCODE -ne 0) { exit 1 } dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal if ($LASTEXITCODE -ne 0) { exit 1 } + # CLI e2e: PowerShell drives `dotnet run` against the native library. + ./dotnet/DdTableForDeal/e2e.ps1 + if ($LASTEXITCODE -ne 0) { exit 1 } # Upload test logs - name: Upload test logs - Windows diff --git a/docs/dotnet_interface.md b/docs/dotnet_interface.md index 5c7027d8..e1f41fe6 100644 --- a/docs/dotnet_interface.md +++ b/docs/dotnet_interface.md @@ -98,6 +98,7 @@ setting `DDS_LIBRARY_PATH` as above: dotnet run --project dotnet/DdTableForDeal/ -- hands/example.pbn dotnet run --project dotnet/DdTableForDeal/ -- --vul ns hands/example.pbn dotnet test dotnet/DdTableForDeal.Tests/ +./dotnet/DdTableForDeal/e2e.sh # Linux/macOS; Windows: e2e.ps1 ``` ## Which native symbols are used diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs new file mode 100644 index 00000000..f323d0a4 --- /dev/null +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -0,0 +1,241 @@ +using System.Text; +using DDS_Core; + +namespace DdTableForDeal; + +/// +/// Testable entry point for the dd_table_for_deal CLI. +/// +public static class DdTableForDealApp +{ + /// Full argv including program name at index 0. + public static int Run( + IReadOnlyList argv, + TextWriter stdout, + TextWriter stderr, + bool stdinIsTty = true, + TextReader? stdin = null) + { + DdTableForDealLib.CliOptions? parsed; + try + { + parsed = DdTableForDealLib.ParseCli(argv, stdinIsTty); + } + catch (ArgumentException ex) + { + stderr.WriteLine(ex.Message); + PrintUsage(stderr, Path.GetFileName(argv[0])); + return 1; + } + + if (parsed is null) + { + PrintUsage(stderr, Path.GetFileName(argv[0])); + return 0; + } + + IReadOnlyList deals; + try + { + deals = DdTableForDealLib.UniqueDeals( + LoadDeals(parsed.Value.DealArg, stderr, stdin)); + } + catch (Exception ex) when (ex is ArgumentException or IOException or InvalidOperationException) + { + stderr.WriteLine(ex.Message); + return 1; + } + + deals = DdTableForDealLib.ApplyDealLimit(deals, parsed.Value.Limit); + + if (deals.Any(d => d.Length >= DdTableForDealLib.PbnDealMax)) + { + stderr.WriteLine( + $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); + return 1; + } + + try + { + using var ctx = new SolverContext(); + var dds = new DDS(); + for (int i = 0; i < deals.Count; i++) + { + if (!ProcessDeal( + ctx, dds, deals[i], i + 1, deals.Count, + parsed.Value.Vulnerable, stdout, stderr)) + { + return 1; + } + } + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException + or BadImageFormatException) + { + stderr.WriteLine( + "Failed to load native DDS library. Build //jni:dds_shared and set " + + "DDS_LIBRARY_PATH to the full path of libdds.dylib / libdds.so / dds.dll."); + stderr.WriteLine(ex.Message); + return 1; + } + + return 0; + } + + private static bool ProcessDeal( + SolverContext ctx, + DDS dds, + string deal, + int dealNo, + int dealCount, + int vulnerable, + TextWriter stdout, + TextWriter stderr) + { + var tableDeal = new DdTableDealPBN { Cards = deal }; + + try + { + ctx.CalcDdTable(tableDeal, out DdTableResults table); + dds.ParAll(in table, vulnerable, out ParResultsMasters sidesMasters); + + string title = dealCount == 1 + ? "dd_table_for_deal:\n" + : $"Deal {dealNo}:\n"; + + stdout.Write(DdTableForDealLib.FormatPbnHand(title, deal)); + stdout.WriteLine(DdTableForDealLib.FormatTable(table)); + stdout.WriteLine(); + + var sides = new ParResultsMaster[] { sidesMasters[0], sidesMasters[1] }; + string? parLine = DdTableForDealLib.FormatParLine(sides); + if (parLine is not null) + { + stdout.WriteLine(parLine); + } + else + { + dds.Par(in table, vulnerable, out ParResults par); + stdout.Write(DdTableForDealLib.FormatParVerbose(par)); + } + + if (dealCount > 1) + stdout.WriteLine(); + return true; + } + catch (Exception ex) + { + stderr.WriteLine($"DDS error: {ex.Message}"); + return false; + } + } + + private static IReadOnlyList LoadDeals( + string arg, TextWriter stderr, TextReader? stdin) + { + if (arg == "-") + { + string? text = ReadPbnStream(stdin ?? Console.In, stderr); + if (text is null) + throw new InvalidOperationException("Cannot read PBN from stdin"); + var deals = DdTableForDealLib.ExtractDealTags(text); + if (deals.Count == 0) + throw new InvalidOperationException("No [Deal \"...\"] tag found in stdin"); + return deals; + } + + string? fileText = ReadPbnFileWorkspaceRelative(arg, stderr); + if (fileText is not null) + { + var deals = DdTableForDealLib.ExtractDealTags(fileText); + if (deals.Count == 0) + throw new InvalidOperationException($"No [Deal \"...\"] tag found in {arg}"); + return deals; + } + + if (DdTableForDealLib.LooksLikePath(arg)) + throw new FileNotFoundException($"Cannot read file: {arg}", arg); + + if (arg.Length >= DdTableForDealLib.PbnDealMax) + throw new ArgumentException( + $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); + + return [arg]; + } + + private static string? ReadPbnFileWorkspaceRelative(string path, TextWriter stderr) + { + if (TryReadPbnFile(path, stderr, out string? text)) + return text; + + string? workspace = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); + if (workspace is not null + && TryReadPbnFile(Path.Combine(workspace, path), stderr, out text)) + { + return text; + } + + return null; + } + + private static bool TryReadPbnFile(string path, TextWriter stderr, out string? text) + { + text = null; + try + { + using var stream = File.OpenRead(path); + using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true); + text = ReadPbnStream(reader, stderr); + return text is not null; + } + catch (IOException) + { + return false; + } + } + + private static string? ReadPbnStream(TextReader reader, TextWriter stderr) + { + var sb = new StringBuilder(); + char[] buffer = new char[4096]; + while (true) + { + int n = reader.Read(buffer, 0, buffer.Length); + if (n <= 0) + break; + sb.Append(buffer, 0, n); + if (sb.Length > DdTableForDealLib.PbnFileMax) + { + stderr.WriteLine( + $"PBN input too large (max {DdTableForDealLib.PbnFileMax} characters)"); + return null; + } + } + return sb.ToString(); + } + + private static void PrintUsage(TextWriter stderr, string prog) + { + stderr.Write( + $"Usage: {prog} [--vul none|both|ns|ew|0|1|2|3] [--limit N] " + + $"\n" + + $" {prog} -h | --help\n" + + "\n" + + "Calculate double-dummy tricks and par for all strains and leads.\n" + + "\n" + + "Arguments:\n" + + " DDS PBN deal string, or path to a .pbn file\n" + + " --vul Vulnerability: none|both|ns|ew or 0|1|2|3" + + " (default: none)\n" + + " --limit Solve only the first N unique deals\n" + + "\n" + + "If stdin is not a terminal, PBN is read from stdin (all [Deal \"...\"] tags).\n" + + "\n" + + "Examples:\n" + + $" {prog} \"N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93\"\n" + + $" {prog} --vul ns hands/example.pbn\n" + + $" {prog} --limit 3 hands/multi_board.pbn\n" + + $" {prog} < hands/example.pbn\n"); + } +} diff --git a/dotnet/DdTableForDeal/Program.cs b/dotnet/DdTableForDeal/Program.cs index 293c6fce..92841fae 100644 --- a/dotnet/DdTableForDeal/Program.cs +++ b/dotnet/DdTableForDeal/Program.cs @@ -1,229 +1,18 @@ -using System.Text; -using DDS_Core; - namespace DdTableForDeal; internal static class Program { private static int Main(string[] args) { - // ParseCli expects argv[0] = program name. var argv = new string[args.Length + 1]; argv[0] = Environment.GetCommandLineArgs()[0]; args.CopyTo(argv, 1); - DdTableForDealLib.CliOptions? parsed; - try - { - parsed = DdTableForDealLib.ParseCli(argv, stdinIsTty: !Console.IsInputRedirected); - } - catch (ArgumentException ex) - { - Console.Error.WriteLine(ex.Message); - PrintUsage(Path.GetFileName(argv[0])); - return 1; - } - - if (parsed is null) - { - PrintUsage(Path.GetFileName(argv[0])); - return 0; - } - - IReadOnlyList deals; - try - { - deals = DdTableForDealLib.UniqueDeals(LoadDeals(parsed.Value.DealArg)); - } - catch (Exception ex) when (ex is ArgumentException or IOException or InvalidOperationException) - { - Console.Error.WriteLine(ex.Message); - return 1; - } - - deals = DdTableForDealLib.ApplyDealLimit(deals, parsed.Value.Limit); - - if (deals.Any(d => d.Length >= DdTableForDealLib.PbnDealMax)) - { - Console.Error.WriteLine( - $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); - return 1; - } - - try - { - using var ctx = new SolverContext(); - var dds = new DDS(); - for (int i = 0; i < deals.Count; i++) - { - if (!ProcessDeal(ctx, dds, deals[i], i + 1, deals.Count, parsed.Value.Vulnerable)) - return 1; - } - } - catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException - or BadImageFormatException) - { - Console.Error.WriteLine( - "Failed to load native DDS library. Build //jni:dds_shared and set " - + "DDS_LIBRARY_PATH to the full path of libdds.dylib / libdds.so / dds.dll."); - Console.Error.WriteLine(ex.Message); - return 1; - } - - return 0; - } - - private static bool ProcessDeal( - SolverContext ctx, - DDS dds, - string deal, - int dealNo, - int dealCount, - int vulnerable) - { - var tableDeal = new DdTableDealPBN { Cards = deal }; - - try - { - ctx.CalcDdTable(tableDeal, out DdTableResults table); - dds.ParAll(in table, vulnerable, out ParResultsMasters sidesMasters); - - string title = dealCount == 1 - ? "dd_table_for_deal:\n" - : $"Deal {dealNo}:\n"; - - Console.Write(DdTableForDealLib.FormatPbnHand(title, deal)); - Console.WriteLine(DdTableForDealLib.FormatTable(table)); - Console.WriteLine(); - - var sides = new ParResultsMaster[] { sidesMasters[0], sidesMasters[1] }; - string? parLine = DdTableForDealLib.FormatParLine(sides); - if (parLine is not null) - { - Console.WriteLine(parLine); - } - else - { - dds.Par(in table, vulnerable, out ParResults par); - Console.Write(DdTableForDealLib.FormatParVerbose(par)); - } - - if (dealCount > 1) - Console.WriteLine(); - return true; - } - catch (Exception ex) - { - Console.Error.WriteLine($"DDS error: {ex.Message}"); - return false; - } - } - - private static IReadOnlyList LoadDeals(string arg) - { - if (arg == "-") - { - string? text = ReadPbnStream(Console.OpenStandardInput()); - if (text is null) - throw new InvalidOperationException("Cannot read PBN from stdin"); - var deals = DdTableForDealLib.ExtractDealTags(text); - if (deals.Count == 0) - throw new InvalidOperationException("No [Deal \"...\"] tag found in stdin"); - return deals; - } - - string? fileText = ReadPbnFileWorkspaceRelative(arg); - if (fileText is not null) - { - var deals = DdTableForDealLib.ExtractDealTags(fileText); - if (deals.Count == 0) - throw new InvalidOperationException($"No [Deal \"...\"] tag found in {arg}"); - return deals; - } - - if (DdTableForDealLib.LooksLikePath(arg)) - throw new FileNotFoundException($"Cannot read file: {arg}", arg); - - if (arg.Length >= DdTableForDealLib.PbnDealMax) - throw new ArgumentException( - $"PBN deal too long (max {DdTableForDealLib.PbnDealMax - 1} characters)"); - - return [arg]; - } - - private static string? ReadPbnFileWorkspaceRelative(string path) - { - if (TryReadPbnFile(path, out string? text)) - return text; - - string? workspace = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); - if (workspace is not null - && TryReadPbnFile(Path.Combine(workspace, path), out text)) - { - return text; - } - - return null; - } - - private static bool TryReadPbnFile(string path, out string? text) - { - text = null; - try - { - using var stream = File.OpenRead(path); - text = ReadPbnStream(stream); - return text is not null; - } - catch (IOException) - { - return false; - } - } - - private static string? ReadPbnStream(Stream stream) - { - using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, leaveOpen: true); - var sb = new StringBuilder(); - char[] buffer = new char[4096]; - while (true) - { - int n = reader.Read(buffer, 0, buffer.Length); - if (n <= 0) - break; - sb.Append(buffer, 0, n); - if (sb.Length > DdTableForDealLib.PbnFileMax) - { - Console.Error.WriteLine( - $"PBN input too large (max {DdTableForDealLib.PbnFileMax} characters)"); - return null; - } - } - return sb.ToString(); - } - - private static void PrintUsage(string prog) - { - Console.Error.Write( - $"Usage: {prog} [--vul none|both|ns|ew|0|1|2|3] [--limit N] " - + $"\n" - + $" {prog} -h | --help\n" - + "\n" - + "Calculate double-dummy tricks and par for all strains and leads.\n" - + "\n" - + "Arguments:\n" - + " DDS PBN deal string, or path to a .pbn file\n" - + " --vul Vulnerability: none|both|ns|ew or 0|1|2|3" - + " (default: none)\n" - + " --limit Solve only the first N unique deals\n" - + "\n" - + "If stdin is not a terminal, PBN is read from stdin (all [Deal \"...\"] tags).\n" - + "\n" - + "Examples:\n" - + $" {prog} \"N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " - + "5.A95432.7632.K6 AKJ9842.K.T8.J93\"\n" - + $" {prog} --vul ns hands/example.pbn\n" - + $" {prog} --limit 3 hands/multi_board.pbn\n" - + $" {prog} < hands/example.pbn\n"); + return DdTableForDealApp.Run( + argv, + Console.Out, + Console.Error, + stdinIsTty: !Console.IsInputRedirected, + stdin: Console.In); } } diff --git a/dotnet/DdTableForDeal/e2e.ps1 b/dotnet/DdTableForDeal/e2e.ps1 new file mode 100644 index 00000000..8620e718 --- /dev/null +++ b/dotnet/DdTableForDeal/e2e.ps1 @@ -0,0 +1,91 @@ +# End-to-end check for the .NET dd_table_for_deal CLI. +# Requires DDS_LIBRARY_PATH pointing at the Bazel-built shared library. +$ErrorActionPreference = "Stop" + +$Root = Resolve-Path (Join-Path $PSScriptRoot "..\..") +Set-Location $Root + +if (-not $env:DDS_LIBRARY_PATH) { + Write-Error "DDS_LIBRARY_PATH is not set (build //jni:dds_shared and export the lib path)" +} +if (-not (Test-Path -LiteralPath $env:DDS_LIBRARY_PATH)) { + Write-Error "DDS_LIBRARY_PATH does not exist: $($env:DDS_LIBRARY_PATH)" +} + +$ExampleDeal = "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 5.A95432.7632.K6 AKJ9842.K.T8.J93" +$Project = "dotnet/DdTableForDeal/" + +function Assert-Contains([string]$Haystack, [string]$Needle) { + if (-not $Haystack.Contains($Needle)) { + Write-Host "Missing expected output: $Needle" -ForegroundColor Red + Write-Host "----- stdout -----" + Write-Host $Haystack + exit 1 + } +} + +function Assert-NotContains([string]$Haystack, [string]$Needle) { + if ($Haystack.Contains($Needle)) { + Write-Host "Unexpected output: $Needle" -ForegroundColor Red + Write-Host "----- stdout -----" + Write-Host $Haystack + exit 1 + } +} + +function Invoke-CliCheck([string]$Label, [string]$Arg) { + Write-Host "==> $Label" + $stdoutPath = [System.IO.Path]::GetTempFileName() + $stderrPath = [System.IO.Path]::GetTempFileName() + try { + $proc = Start-Process -FilePath "dotnet" ` + -ArgumentList @("run", "--project", $Project, "--", $Arg) ` + -WorkingDirectory $Root ` + -NoNewWindow -Wait -PassThru ` + -RedirectStandardOutput $stdoutPath ` + -RedirectStandardError $stderrPath + $out = Get-Content -Raw -LiteralPath $stdoutPath + $err = Get-Content -Raw -LiteralPath $stderrPath + if ($null -eq $out) { $out = "" } + if ($null -eq $err) { $err = "" } + + if ($proc.ExitCode -ne 0) { + Write-Host "dotnet run failed (exit $($proc.ExitCode))" -ForegroundColor Red + Write-Host "----- stderr -----" + Write-Host $err + Write-Host "----- stdout -----" + Write-Host $out + exit 1 + } + if ($err.Contains("DDS error:") -or $err.Contains("Failed to load native DDS library")) { + Write-Host "Solver error on stderr:" -ForegroundColor Red + Write-Host $err + exit 1 + } + + Assert-Contains $out "dd_table_for_deal:" + Assert-Contains $out "North" + Assert-Contains $out " NT 4 4 8 8" + Assert-Contains $out " S 3 3 10 10" + Assert-Contains $out " H 9 9 4 4" + Assert-Contains $out " D 8 8 4 4" + Assert-Contains $out " C 3 3 9 9" + Assert-Contains $out "Par: NS 5Hx -2 -300" + Assert-NotContains $out "NS score:" + + $northPos = $out.IndexOf("North") + $parPos = $out.IndexOf("Par:") + if ($northPos -lt 0 -or $parPos -lt 0 -or $northPos -ge $parPos) { + Write-Host "Expected 'North' before 'Par:' in output" -ForegroundColor Red + exit 1 + } + } + finally { + Remove-Item -LiteralPath $stdoutPath, $stderrPath -ErrorAction SilentlyContinue + } +} + +Invoke-CliCheck "inline PBN deal" $ExampleDeal +Invoke-CliCheck "hands/example.pbn" "hands/example.pbn" + +Write-Host "DdTableForDeal e2e OK" diff --git a/dotnet/DdTableForDeal/e2e.sh b/dotnet/DdTableForDeal/e2e.sh new file mode 100755 index 00000000..f4cf1a01 --- /dev/null +++ b/dotnet/DdTableForDeal/e2e.sh @@ -0,0 +1,99 @@ +#!/usr/bin/env bash +# End-to-end check for the .NET dd_table_for_deal CLI. +# Requires DDS_LIBRARY_PATH pointing at the Bazel-built shared library. +# +# bazelisk build //jni:dds_shared +# export DDS_LIBRARY_PATH="$(bazelisk info bazel-bin)/jni/libdds.dylib" # .so / dds.dll +# ./dotnet/DdTableForDeal/e2e.sh +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cd "$ROOT" + +if [[ -z "${DDS_LIBRARY_PATH:-}" ]]; then + echo "DDS_LIBRARY_PATH is not set (build //jni:dds_shared and export the lib path)" >&2 + exit 1 +fi +if [[ ! -f "$DDS_LIBRARY_PATH" ]]; then + echo "DDS_LIBRARY_PATH does not exist: $DDS_LIBRARY_PATH" >&2 + exit 1 +fi + +EXAMPLE_DEAL='N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 5.A95432.7632.K6 AKJ9842.K.T8.J93' +PROJECT=dotnet/DdTableForDeal/ + +assert_contains() { + local haystack=$1 + local needle=$2 + if [[ "$haystack" != *"$needle"* ]]; then + echo "Missing expected output: ${needle}" >&2 + echo "----- stdout -----" >&2 + printf '%s\n' "$haystack" >&2 + exit 1 + fi +} + +assert_not_contains() { + local haystack=$1 + local needle=$2 + if [[ "$haystack" == *"$needle"* ]]; then + echo "Unexpected output: ${needle}" >&2 + echo "----- stdout -----" >&2 + printf '%s\n' "$haystack" >&2 + exit 1 + fi +} + +run_and_check() { + local label=$1 + shift + echo "==> ${label}" + + local err_file out rc + err_file="$(mktemp)" + set +e + out="$(dotnet run --project "$PROJECT" -- "$@" 2>"$err_file")" + rc=$? + set -e + local err + err="$(cat "$err_file")" + rm -f "$err_file" + + if [[ $rc -ne 0 ]]; then + echo "dotnet run failed (exit ${rc})" >&2 + echo "----- stderr -----" >&2 + printf '%s\n' "$err" >&2 + echo "----- stdout -----" >&2 + printf '%s\n' "$out" >&2 + exit 1 + fi + if [[ "$err" == *"DDS error:"* || "$err" == *"Failed to load native DDS library"* ]]; then + echo "Solver error on stderr:" >&2 + printf '%s\n' "$err" >&2 + exit 1 + fi + + assert_contains "$out" 'dd_table_for_deal:' + assert_contains "$out" 'North' + assert_contains "$out" ' NT 4 4 8 8' + assert_contains "$out" ' S 3 3 10 10' + assert_contains "$out" ' H 9 9 4 4' + assert_contains "$out" ' D 8 8 4 4' + assert_contains "$out" ' C 3 3 9 9' + assert_contains "$out" 'Par: NS 5Hx -2 -300' + assert_not_contains "$out" 'NS score:' + + local before_north before_par + before_north=${out%%North*} + before_par=${out%%Par:*} + if [[ "$before_north" == "$out" || "$before_par" == "$out" \ + || ${#before_north} -ge ${#before_par} ]]; then + echo "Expected 'North' before 'Par:' in output" >&2 + exit 1 + fi +} + +run_and_check "inline PBN deal" "$EXAMPLE_DEAL" +run_and_check "hands/example.pbn" hands/example.pbn + +echo "DdTableForDeal e2e OK" diff --git a/examples/README b/examples/README index b27e57ee..d7e29aa7 100644 --- a/examples/README +++ b/examples/README @@ -23,6 +23,7 @@ Python `dd_table_for_deal` (see `python/examples/`): dotnet run --project dotnet/DdTableForDeal/ -- --vul ns hands/example.pbn dotnet run --project dotnet/DdTableForDeal/ -- --limit 3 hands/multi_board.pbn dotnet test dotnet/DdTableForDeal.Tests/ + ./dotnet/DdTableForDeal/e2e.sh # requires DDS_LIBRARY_PATH; Windows: e2e.ps1 Available examples: - analyse_all_plays_bin, analyse_all_plays_pbn From eda89783f3b4f20fddfb7456fc64d66705183522 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Fri, 14 Aug 2026 22:33:28 +0200 Subject: [PATCH 25/35] Fix Windows e2e argument splitting for spaced PBN deals. Start-Process -ArgumentList joins without quoting; use ProcessStartInfo.ArgumentList instead. Co-authored-by: Cursor --- dotnet/DdTableForDeal/e2e.ps1 | 96 +++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/dotnet/DdTableForDeal/e2e.ps1 b/dotnet/DdTableForDeal/e2e.ps1 index 8620e718..e2078aee 100644 --- a/dotnet/DdTableForDeal/e2e.ps1 +++ b/dotnet/DdTableForDeal/e2e.ps1 @@ -35,53 +35,61 @@ function Assert-NotContains([string]$Haystack, [string]$Needle) { function Invoke-CliCheck([string]$Label, [string]$Arg) { Write-Host "==> $Label" - $stdoutPath = [System.IO.Path]::GetTempFileName() - $stderrPath = [System.IO.Path]::GetTempFileName() - try { - $proc = Start-Process -FilePath "dotnet" ` - -ArgumentList @("run", "--project", $Project, "--", $Arg) ` - -WorkingDirectory $Root ` - -NoNewWindow -Wait -PassThru ` - -RedirectStandardOutput $stdoutPath ` - -RedirectStandardError $stderrPath - $out = Get-Content -Raw -LiteralPath $stdoutPath - $err = Get-Content -Raw -LiteralPath $stderrPath - if ($null -eq $out) { $out = "" } - if ($null -eq $err) { $err = "" } + # Do not use Start-Process -ArgumentList with a spaced PBN string: PowerShell + # joins the array into one command line without quoting, so the deal is split + # into multiple argv entries ("Only one deal argument is allowed"). + $psi = [System.Diagnostics.ProcessStartInfo]::new() + $psi.FileName = "dotnet" + $psi.WorkingDirectory = "$Root" + $psi.UseShellExecute = $false + $psi.RedirectStandardOutput = $true + $psi.RedirectStandardError = $true + [void]$psi.ArgumentList.Add("run") + [void]$psi.ArgumentList.Add("--project") + [void]$psi.ArgumentList.Add($Project) + [void]$psi.ArgumentList.Add("--") + [void]$psi.ArgumentList.Add($Arg) - if ($proc.ExitCode -ne 0) { - Write-Host "dotnet run failed (exit $($proc.ExitCode))" -ForegroundColor Red - Write-Host "----- stderr -----" - Write-Host $err - Write-Host "----- stdout -----" - Write-Host $out - exit 1 - } - if ($err.Contains("DDS error:") -or $err.Contains("Failed to load native DDS library")) { - Write-Host "Solver error on stderr:" -ForegroundColor Red - Write-Host $err - exit 1 - } + $proc = [System.Diagnostics.Process]::new() + $proc.StartInfo = $psi + [void]$proc.Start() + $outTask = $proc.StandardOutput.ReadToEndAsync() + $errTask = $proc.StandardError.ReadToEndAsync() + $proc.WaitForExit() + $out = $outTask.GetAwaiter().GetResult() + $err = $errTask.GetAwaiter().GetResult() + if ($null -eq $out) { $out = "" } + if ($null -eq $err) { $err = "" } - Assert-Contains $out "dd_table_for_deal:" - Assert-Contains $out "North" - Assert-Contains $out " NT 4 4 8 8" - Assert-Contains $out " S 3 3 10 10" - Assert-Contains $out " H 9 9 4 4" - Assert-Contains $out " D 8 8 4 4" - Assert-Contains $out " C 3 3 9 9" - Assert-Contains $out "Par: NS 5Hx -2 -300" - Assert-NotContains $out "NS score:" - - $northPos = $out.IndexOf("North") - $parPos = $out.IndexOf("Par:") - if ($northPos -lt 0 -or $parPos -lt 0 -or $northPos -ge $parPos) { - Write-Host "Expected 'North' before 'Par:' in output" -ForegroundColor Red - exit 1 - } + if ($proc.ExitCode -ne 0) { + Write-Host "dotnet run failed (exit $($proc.ExitCode))" -ForegroundColor Red + Write-Host "----- stderr -----" + Write-Host $err + Write-Host "----- stdout -----" + Write-Host $out + exit 1 + } + if ($err.Contains("DDS error:") -or $err.Contains("Failed to load native DDS library")) { + Write-Host "Solver error on stderr:" -ForegroundColor Red + Write-Host $err + exit 1 } - finally { - Remove-Item -LiteralPath $stdoutPath, $stderrPath -ErrorAction SilentlyContinue + + Assert-Contains $out "dd_table_for_deal:" + Assert-Contains $out "North" + Assert-Contains $out " NT 4 4 8 8" + Assert-Contains $out " S 3 3 10 10" + Assert-Contains $out " H 9 9 4 4" + Assert-Contains $out " D 8 8 4 4" + Assert-Contains $out " C 3 3 9 9" + Assert-Contains $out "Par: NS 5Hx -2 -300" + Assert-NotContains $out "NS score:" + + $northPos = $out.IndexOf("North") + $parPos = $out.IndexOf("Par:") + if ($northPos -lt 0 -or $parPos -lt 0 -or $northPos -ge $parPos) { + Write-Host "Expected 'North' before 'Par:' in output" -ForegroundColor Red + exit 1 } } From bea024a17064178a18bee29aada02feaa8ccec2a Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 05:54:13 +0200 Subject: [PATCH 26/35] Split Windows .NET binding tests into their own CI workflow. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keeps CI – Windows focused on Bazel while Windows .NET builds dds.dll and runs the managed tests and e2e. Co-authored-by: Cursor --- .github/workflows/ci_windows.yml | 39 +------------- .github/workflows/ci_windows_dotnet.yml | 71 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/ci_windows_dotnet.yml diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index cc64dd0c..10cf9bb4 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -56,44 +56,7 @@ jobs: - name: Run all tests run: bazelisk test --config=opt --verbose_failures //... - # .NET binding — this is the platform the shim retarget can regress. - # Windows was previously the only place the binding worked, via - # dds_native.dll and dds_api.hpp's dds_* symbols; it now binds dds_c_* - # in the Bazel-built dds.dll, a path no other test covers (the JNI smoke - # tests and export_set_test are target_compatible_with-excluded here). - # Pinned to 8.0.x to match what DDS_Core targets. - - name: Setup .NET - uses: actions/setup-dotnet@v4 - with: - dotnet-version: "8.0.x" - - # bazel-bin is a configuration-dependent convenience symlink, so ask Bazel - # for it rather than hardcoding a path that a different --config would move. - - name: Test .NET binding - shell: pwsh - run: | - bazelisk build --verbose_failures //jni:dds_shared - if ($LASTEXITCODE -ne 0) { exit 1 } - $binDir = bazelisk info bazel-bin - $env:DDS_LIBRARY_PATH = Join-Path $binDir "jni\dds.dll" - if (-not (Test-Path $env:DDS_LIBRARY_PATH)) { - Write-Host "native library not found at $env:DDS_LIBRARY_PATH" - exit 1 - } - # Both configurations: ThrowIfError is unconditional, so a regression to - # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. - dotnet test dotnet/DDS_Core.Tests/ --verbosity normal - if ($LASTEXITCODE -ne 0) { exit 1 } - dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal - if ($LASTEXITCODE -ne 0) { exit 1 } - # DdTableForDeal unit helpers (no native library required). - dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal - if ($LASTEXITCODE -ne 0) { exit 1 } - dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal - if ($LASTEXITCODE -ne 0) { exit 1 } - # CLI e2e: PowerShell drives `dotnet run` against the native library. - ./dotnet/DdTableForDeal/e2e.ps1 - if ($LASTEXITCODE -ne 0) { exit 1 } + # .NET binding lives in CI – Windows .NET (ci_windows_dotnet.yml). # Upload test logs - name: Upload test logs - Windows diff --git a/.github/workflows/ci_windows_dotnet.yml b/.github/workflows/ci_windows_dotnet.yml new file mode 100644 index 00000000..6e59e3c2 --- /dev/null +++ b/.github/workflows/ci_windows_dotnet.yml @@ -0,0 +1,71 @@ +# .NET binding CI on Windows (Win64 ABI + Bazel-built dds.dll). +# Split out of CI – Windows so the managed tests can run without waiting on +# the full Bazel //... build/test matrix. +name: CI – Windows .NET + +on: + pull_request: + branches: [main, develop] + workflow_dispatch: {} + +jobs: + build_and_test: + runs-on: windows-latest + timeout-minutes: 20 + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Setup Bazelisk + uses: ./.github/actions/setup-bazelisk + + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "8.0.x" + + # Windows is the platform the shim retarget can regress: the binding now + # loads Bazel-built dds.dll via dds_c_*, a path no other test covers (JNI + # smoke tests and export_set_test are target_compatible_with-excluded here). + # bazel-bin is configuration-dependent, so ask Bazel for it rather than + # hardcoding a path that a different --config would move. + # Pinned to 8.0.x to match what DDS_Core targets. + - name: Build native library and test .NET binding + shell: pwsh + run: | + $max = 3 + for ($i = 1; $i -le $max; $i++) { + bazelisk build --verbose_failures //jni:dds_shared + if ($LASTEXITCODE -eq 0) { break } + if ($i -lt $max) { + Write-Host "Build failed (attempt $i/$max). Retrying in 20s..." + Start-Sleep -Seconds 20 + bazelisk shutdown || $true + } else { + exit 1 + } + } + + $binDir = bazelisk info bazel-bin + $env:DDS_LIBRARY_PATH = Join-Path $binDir "jni\dds.dll" + if (-not (Test-Path $env:DDS_LIBRARY_PATH)) { + Write-Host "native library not found at $env:DDS_LIBRARY_PATH" + exit 1 + } + + # Both configurations: ThrowIfError is unconditional, so a regression to + # [Conditional("DEBUG")] only surfaces in Release. See dotnet-binding.md. + dotnet test dotnet/DDS_Core.Tests/ --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + dotnet test dotnet/DDS_Core.Tests/ -c Release --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + + # DdTableForDeal unit helpers (no native library required). + dotnet test dotnet/DdTableForDeal.Tests/ --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + dotnet test dotnet/DdTableForDeal.Tests/ -c Release --verbosity normal + if ($LASTEXITCODE -ne 0) { exit 1 } + + # CLI e2e: PowerShell drives `dotnet run` against the native library. + ./dotnet/DdTableForDeal/e2e.ps1 + if ($LASTEXITCODE -ne 0) { exit 1 } From 4aac90372c3f897d4885f2383951f7112baf4fa5 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sat, 15 Aug 2026 10:40:08 +0100 Subject: [PATCH 27/35] fix: updates actions/setup-dotnet to version 5. --- .github/workflows/ci_linux.yml | 2 +- .github/workflows/ci_macos.yml | 2 +- .github/workflows/ci_windows.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci_linux.yml b/.github/workflows/ci_linux.yml index 54008e7b..533685d2 100644 --- a/.github/workflows/ci_linux.yml +++ b/.github/workflows/ci_linux.yml @@ -73,7 +73,7 @@ jobs: # targets; the test project's RollForward only matters where no 8.0 # runtime exists, which is not the case here. - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: "8.0.x" diff --git a/.github/workflows/ci_macos.yml b/.github/workflows/ci_macos.yml index a99090af..ca785136 100644 --- a/.github/workflows/ci_macos.yml +++ b/.github/workflows/ci_macos.yml @@ -55,7 +55,7 @@ jobs: # x86-64 and Windows covers Win64, so all three ABIs are exercised. # Pinned to 8.0.x to match what DDS_Core targets. - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: "8.0.x" diff --git a/.github/workflows/ci_windows.yml b/.github/workflows/ci_windows.yml index a3a1af2e..9ef43b18 100644 --- a/.github/workflows/ci_windows.yml +++ b/.github/workflows/ci_windows.yml @@ -63,7 +63,7 @@ jobs: # tests and export_set_test are target_compatible_with-excluded here). # Pinned to 8.0.x to match what DDS_Core targets. - name: Setup .NET - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: "8.0.x" From 3f37f2d2f6634850c9b77a4966f5ecf08ffa2f35 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sat, 15 Aug 2026 10:45:18 +0100 Subject: [PATCH 28/35] fix: updates mystery comment to say what is tested instead of quoting the bug report. --- library/tests/trans_table/trans_table_s_test.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/library/tests/trans_table/trans_table_s_test.cpp b/library/tests/trans_table/trans_table_s_test.cpp index 10f7d94e..cda883cc 100644 --- a/library/tests/trans_table/trans_table_s_test.cpp +++ b/library/tests/trans_table/trans_table_s_test.cpp @@ -33,11 +33,8 @@ static void CreateTestWinRanks(unsigned short win_ranks[DDS_SUITS]) { win_ranks[3] = 0x8888; // Clubs } -// Regression: reset_memory() after return_all_memory() must be inert. -// return_all_memory() frees pw_/pn_/pl_ and clears tt_in_use_; without the -// guard in TransTableS::reset_memory(), init_tt() dereferences the freed pools -// (pw_[0]) and segfaults. TransTableL::reset_memory() already guards the -// equivalent case with `pool_ == nullptr`. +// Test that verifies that calling reset_memory() after return_all_memory() +// is inert and does not cause a crash. TEST(TransTableSMemoryTest, ResetAfterReturnAllMemoryIsInert) { TransTableS tt; From 21c2d8956df0955c84892698c4005763bf96bcfe Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 11:55:38 +0200 Subject: [PATCH 29/35] Clarify PBN load errors and harden ConvertPbn against malformed input. Throw on oversized PBN instead of misreporting missing files, catch permission failures, and ignore out-of-range suits/hands in ConvertPbn. Co-authored-by: Cursor --- .../DdTableForDealLibTests.cs | 45 ++++++++++ .../DdTableForDeal.Tests/PbnLoadErrorTests.cs | 82 +++++++++++++++++++ dotnet/DdTableForDeal/DdTableForDealApp.cs | 33 ++++---- dotnet/DdTableForDeal/DdTableForDealLib.cs | 15 +++- 4 files changed, 156 insertions(+), 19 deletions(-) create mode 100644 dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs diff --git a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs index 30dc91ae..339bb074 100644 --- a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs +++ b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs @@ -310,3 +310,48 @@ public void EndsWithBlankLineAfterDiagram() Assert.EndsWith("\n\n", text); } } + +public class ConvertPbnTests +{ + [Theory] + [InlineData("")] + [InlineData("N")] + [InlineData("N:")] + [InlineData("12")] + [InlineData("abc")] + public void ShortOrMalformedDealStringsDoNotRaise(string deal) + { + var remain = DdTableForDealLib.ConvertPbn(deal); + Assert.Equal(4, remain.GetLength(0)); + Assert.Equal(4, remain.GetLength(1)); + for (int h = 0; h < 4; h++) + for (int s = 0; s < 4; s++) + Assert.Equal(0u, remain[h, s]); + } + + [Fact] + public void ExtraSuitsAndHandsDoNotRaise() + { + // Extra '.' beyond 4 suits and extra ' ' beyond 4 hands must not + // IndexOutOfRange when indexing remain[hand, suit]. + const string deal = "N:A.K.Q.J.T W.E.S.T.X E.A.S.T.Y S.O.U.T.H Z.Z.Z.Z"; + var remain = DdTableForDealLib.ConvertPbn(deal); + Assert.NotNull(remain); + Assert.Equal(4, remain.GetLength(0)); + Assert.Equal(4, remain.GetLength(1)); + } + + [Fact] + public void ValidDealParsesCardBitmasks() + { + const string deal = + "N:73.QJT.AQ54.T752 QT6.876.KJ9.AQ84 " + + "5.A95432.7632.K6 AKJ9842.K.T8.J93"; + var remain = DdTableForDealLib.ConvertPbn(deal); + + // North's spades: 73 + Assert.Equal(0x0080u | 0x0008u, remain[0, 0]); + // North's hearts: QJT + Assert.Equal(0x1000u | 0x0800u | 0x0400u, remain[0, 1]); + } +} diff --git a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs new file mode 100644 index 00000000..3d4f5c53 --- /dev/null +++ b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs @@ -0,0 +1,82 @@ +namespace DdTableForDeal.Tests; + +/// +/// Error-path coverage for CLI PBN loading (Copilot review on PR #321). +/// +public class PbnLoadErrorTests +{ + [Fact] + public void Run_OversizedStdin_ReportsSizeNotGenericStdinFailure() + { + string huge = new('x', DdTableForDealLib.PbnFileMax + 1); + var stdout = new StringWriter(); + var stderr = new StringWriter(); + + int rc = DdTableForDealApp.Run( + ["dd_table_for_deal", "-"], + stdout, + stderr, + stdinIsTty: false, + stdin: new StringReader(huge)); + + Assert.Equal(1, rc); + Assert.Contains("PBN input too large", stderr.ToString()); + Assert.DoesNotContain("Cannot read PBN from stdin", stderr.ToString()); + } + + [Fact] + public void Run_OversizedPbnFile_ReportsSizeNotCannotReadFile() + { + string path = Path.Combine(Path.GetTempPath(), $"dds_oversized_{Guid.NewGuid():N}.pbn"); + File.WriteAllText(path, new string('x', DdTableForDealLib.PbnFileMax + 1)); + try + { + var stdout = new StringWriter(); + var stderr = new StringWriter(); + + int rc = DdTableForDealApp.Run( + ["dd_table_for_deal", path], + stdout, + stderr); + + Assert.Equal(1, rc); + Assert.Contains("PBN input too large", stderr.ToString()); + Assert.DoesNotContain("Cannot read file:", stderr.ToString()); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public void Run_UnreadablePbnFile_ReportsCannotReadFile() + { + // UnauthorizedAccessException is hard to force portably on Windows CI; + // cover the Unix permission-denied path which hits the same catch. + if (OperatingSystem.IsWindows()) + return; + + string path = Path.Combine(Path.GetTempPath(), $"dds_denied_{Guid.NewGuid():N}.pbn"); + File.WriteAllText(path, "[Deal \"N:..\"]\n"); + File.SetUnixFileMode(path, UnixFileMode.None); + try + { + var stdout = new StringWriter(); + var stderr = new StringWriter(); + + int rc = DdTableForDealApp.Run( + ["dd_table_for_deal", path], + stdout, + stderr); + + Assert.Equal(1, rc); + Assert.Contains("Cannot read file:", stderr.ToString()); + } + finally + { + File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite); + File.Delete(path); + } + } +} diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs index f323d0a4..29756613 100644 --- a/dotnet/DdTableForDeal/DdTableForDealApp.cs +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -38,9 +38,11 @@ public static int Run( try { deals = DdTableForDealLib.UniqueDeals( - LoadDeals(parsed.Value.DealArg, stderr, stdin)); + LoadDeals(parsed.Value.DealArg, stdin)); } - catch (Exception ex) when (ex is ArgumentException or IOException or InvalidOperationException) + catch (Exception ex) when (ex is ArgumentException or IOException + or InvalidOperationException + or UnauthorizedAccessException) { stderr.WriteLine(ex.Message); return 1; @@ -131,20 +133,18 @@ private static bool ProcessDeal( } private static IReadOnlyList LoadDeals( - string arg, TextWriter stderr, TextReader? stdin) + string arg, TextReader? stdin) { if (arg == "-") { - string? text = ReadPbnStream(stdin ?? Console.In, stderr); - if (text is null) - throw new InvalidOperationException("Cannot read PBN from stdin"); + string text = ReadPbnStream(stdin ?? Console.In); var deals = DdTableForDealLib.ExtractDealTags(text); if (deals.Count == 0) throw new InvalidOperationException("No [Deal \"...\"] tag found in stdin"); return deals; } - string? fileText = ReadPbnFileWorkspaceRelative(arg, stderr); + string? fileText = ReadPbnFileWorkspaceRelative(arg); if (fileText is not null) { var deals = DdTableForDealLib.ExtractDealTags(fileText); @@ -163,14 +163,14 @@ private static IReadOnlyList LoadDeals( return [arg]; } - private static string? ReadPbnFileWorkspaceRelative(string path, TextWriter stderr) + private static string? ReadPbnFileWorkspaceRelative(string path) { - if (TryReadPbnFile(path, stderr, out string? text)) + if (TryReadPbnFile(path, out string? text)) return text; string? workspace = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); if (workspace is not null - && TryReadPbnFile(Path.Combine(workspace, path), stderr, out text)) + && TryReadPbnFile(Path.Combine(workspace, path), out text)) { return text; } @@ -178,23 +178,23 @@ private static IReadOnlyList LoadDeals( return null; } - private static bool TryReadPbnFile(string path, TextWriter stderr, out string? text) + private static bool TryReadPbnFile(string path, out string? text) { text = null; try { using var stream = File.OpenRead(path); using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true); - text = ReadPbnStream(reader, stderr); - return text is not null; + text = ReadPbnStream(reader); + return true; } - catch (IOException) + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return false; } } - private static string? ReadPbnStream(TextReader reader, TextWriter stderr) + private static string ReadPbnStream(TextReader reader) { var sb = new StringBuilder(); char[] buffer = new char[4096]; @@ -206,9 +206,8 @@ private static bool TryReadPbnFile(string path, TextWriter stderr, out string? t sb.Append(buffer, 0, n); if (sb.Length > DdTableForDealLib.PbnFileMax) { - stderr.WriteLine( + throw new InvalidOperationException( $"PBN input too large (max {DdTableForDealLib.PbnFileMax} characters)"); - return null; } } return sb.ToString(); diff --git a/dotnet/DdTableForDeal/DdTableForDealLib.cs b/dotnet/DdTableForDeal/DdTableForDealLib.cs index 50347030..457ba32d 100644 --- a/dotnet/DdTableForDeal/DdTableForDealLib.cs +++ b/dotnet/DdTableForDeal/DdTableForDealLib.cs @@ -314,6 +314,11 @@ public static string FormatParVerbose(in ParResults par) int card = IsCard(ch); if (card != 0) { + if (handRelFirst is < 0 or > 3 || suitInHand is < 0 or > 3) + { + bp++; + continue; + } int hand = first switch { 0 => handRelFirst, @@ -325,11 +330,17 @@ public static string FormatParVerbose(in ParResults par) } else if (ch == '.') { - suitInHand++; + if (suitInHand < 3) + suitInHand++; + else + suitInHand = 4; } else if (ch == ' ') { - handRelFirst++; + if (handRelFirst < 3) + handRelFirst++; + else + handRelFirst = 4; suitInHand = 0; } bp++; From 6e9f6b68e22a5c950d29572615d89c68d8e0f075 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 12:06:44 +0200 Subject: [PATCH 30/35] Address Copilot notes on ConvertPbn limit and path errors. Use PbnDealMax for the scan bound and treat ArgumentException / NotSupportedException from OpenRead as unreadable file paths. Co-authored-by: Cursor --- .../DdTableForDealLibTests.cs | 12 ++++++++++++ .../DdTableForDeal.Tests/PbnLoadErrorTests.cs | 19 +++++++++++++++++++ dotnet/DdTableForDeal/DdTableForDealApp.cs | 5 ++++- dotnet/DdTableForDeal/DdTableForDealLib.cs | 2 +- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs index 339bb074..d13e7565 100644 --- a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs +++ b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs @@ -354,4 +354,16 @@ public void ValidDealParsesCardBitmasks() // North's hearts: QJT Assert.Equal(0x1000u | 0x0800u | 0x0400u, remain[0, 1]); } + + [Fact] + public void StopsScanningAtPbnDealMax() + { + // Ace at index PbnDealMax is past the scan limit (bp < PbnDealMax). + string deal = "N:" + new string('.', DdTableForDealLib.PbnDealMax - 2) + "A"; + Assert.Equal(DdTableForDealLib.PbnDealMax + 1, deal.Length); + Assert.Equal('A', deal[DdTableForDealLib.PbnDealMax]); + + var remain = DdTableForDealLib.ConvertPbn(deal); + Assert.Equal(0u, remain[0, 0]); + } } diff --git a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs index 3d4f5c53..08cb8845 100644 --- a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs +++ b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs @@ -79,4 +79,23 @@ public void Run_UnreadablePbnFile_ReportsCannotReadFile() File.Delete(path); } } + + [Fact] + public void Run_InvalidPathCharacters_ReportsCannotReadFile() + { + // Null in the path throws ArgumentException from File.OpenRead on every OS. + string path = "bad\0name.pbn"; + Assert.True(DdTableForDealLib.LooksLikePath(path)); + + var stdout = new StringWriter(); + var stderr = new StringWriter(); + + int rc = DdTableForDealApp.Run( + ["dd_table_for_deal", path], + stdout, + stderr); + + Assert.Equal(1, rc); + Assert.Contains("Cannot read file:", stderr.ToString()); + } } diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs index 29756613..c7003f49 100644 --- a/dotnet/DdTableForDeal/DdTableForDealApp.cs +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -188,7 +188,10 @@ private static bool TryReadPbnFile(string path, out string? text) text = ReadPbnStream(reader); return true; } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + catch (Exception ex) when (ex is IOException + or UnauthorizedAccessException + or ArgumentException + or NotSupportedException) { return false; } diff --git a/dotnet/DdTableForDeal/DdTableForDealLib.cs b/dotnet/DdTableForDeal/DdTableForDealLib.cs index 457ba32d..60bb55d8 100644 --- a/dotnet/DdTableForDeal/DdTableForDealLib.cs +++ b/dotnet/DdTableForDeal/DdTableForDealLib.cs @@ -308,7 +308,7 @@ public static string FormatParVerbose(in ParResults par) int handRelFirst = 0; int suitInHand = 0; - while (bp < 80 && bp < pbnDeal.Length) + while (bp < PbnDealMax && bp < pbnDeal.Length) { char ch = pbnDeal[bp]; int card = IsCard(ch); From 74fe64be72a6ae97ba48d35e53440d73d304e703 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 12:23:26 +0200 Subject: [PATCH 31/35] Guard workspace-relative Path.Combine for invalid path args. Catch ArgumentException from Path.Combine when BUILD_WORKSPACE_DIRECTORY is set so malformed paths stay on the "Cannot read file" CLI path. Co-authored-by: Cursor --- .../DdTableForDeal.Tests/PbnLoadErrorTests.cs | 30 +++++++++++++++++++ dotnet/DdTableForDeal/DdTableForDealApp.cs | 17 ++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs index 08cb8845..eb557264 100644 --- a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs +++ b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs @@ -98,4 +98,34 @@ public void Run_InvalidPathCharacters_ReportsCannotReadFile() Assert.Equal(1, rc); Assert.Contains("Cannot read file:", stderr.ToString()); } + + [Fact] + public void Run_InvalidPathCharacters_WithWorkspaceEnv_ReportsCannotReadFile() + { + // Path.Combine(workspace, path) throws ArgumentException when path has + // invalid characters; that must not escape as a low-level CLI error. + string path = "bad\0name.pbn"; + string? previous = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); + Environment.SetEnvironmentVariable( + "BUILD_WORKSPACE_DIRECTORY", + Path.GetTempPath()); + try + { + var stdout = new StringWriter(); + var stderr = new StringWriter(); + + int rc = DdTableForDealApp.Run( + ["dd_table_for_deal", path], + stdout, + stderr); + + Assert.Equal(1, rc); + Assert.Contains("Cannot read file:", stderr.ToString()); + Assert.DoesNotContain("ArgumentException", stderr.ToString()); + } + finally + { + Environment.SetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY", previous); + } + } } diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs index c7003f49..3c81db55 100644 --- a/dotnet/DdTableForDeal/DdTableForDealApp.cs +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -169,13 +169,22 @@ private static IReadOnlyList LoadDeals( return text; string? workspace = Environment.GetEnvironmentVariable("BUILD_WORKSPACE_DIRECTORY"); - if (workspace is not null - && TryReadPbnFile(Path.Combine(workspace, path), out text)) + if (workspace is null) + return null; + + string combined; + try { - return text; + combined = Path.Combine(workspace, path); + } + catch (ArgumentException) + { + // Invalid path characters (or other Path.Combine argument errors) + // should fall through to LoadDeals' "Cannot read file" handling. + return null; } - return null; + return TryReadPbnFile(combined, out text) ? text : null; } private static bool TryReadPbnFile(string path, out string? text) From 9513ad432a0f6040c44cac907143462b2536af8d Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 12:35:55 +0200 Subject: [PATCH 32/35] Distinguish DDS failures from unexpected ProcessDeal errors. Report InvalidOperationException as DDS error and other exceptions as Unexpected error; drop the stale PR number from the load-error test docs. Co-authored-by: Cursor --- .../DdTableForDeal.Tests/PbnLoadErrorTests.cs | 2 +- .../ProcessDealErrorTests.cs | 27 +++++++++++++++++++ dotnet/DdTableForDeal/DdTableForDealApp.cs | 11 +++++++- 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs diff --git a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs index eb557264..269cb3e3 100644 --- a/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs +++ b/dotnet/DdTableForDeal.Tests/PbnLoadErrorTests.cs @@ -1,7 +1,7 @@ namespace DdTableForDeal.Tests; /// -/// Error-path coverage for CLI PBN loading (Copilot review on PR #321). +/// Error-path coverage for CLI PBN loading. /// public class PbnLoadErrorTests { diff --git a/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs b/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs new file mode 100644 index 00000000..a3ca7016 --- /dev/null +++ b/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs @@ -0,0 +1,27 @@ +namespace DdTableForDeal.Tests; + +/// +/// Distinguishes solver failures from unexpected runtime errors in ProcessDeal. +/// +public class ProcessDealErrorTests +{ + [Fact] + public void FormatProcessDealFailure_InvalidOperation_IsDdsError() + { + string message = DdTableForDealApp.FormatProcessDealFailure( + new InvalidOperationException("CalcDdTable failed with code -1: PBN string error")); + + Assert.Equal( + "DDS error: CalcDdTable failed with code -1: PBN string error", + message); + } + + [Fact] + public void FormatProcessDealFailure_OtherException_IsUnexpectedError() + { + string message = DdTableForDealApp.FormatProcessDealFailure( + new IOException("simulated stdout failure")); + + Assert.Equal("Unexpected error: simulated stdout failure", message); + } +} diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs index 3c81db55..6385221e 100644 --- a/dotnet/DdTableForDeal/DdTableForDealApp.cs +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -127,11 +127,20 @@ private static bool ProcessDeal( } catch (Exception ex) { - stderr.WriteLine($"DDS error: {ex.Message}"); + stderr.WriteLine(FormatProcessDealFailure(ex)); return false; } } + /// + /// Maps ProcessDeal exceptions to CLI stderr lines: DDS return-code failures + /// surface as InvalidOperationException; everything else is unexpected. + /// + public static string FormatProcessDealFailure(Exception ex) => + ex is InvalidOperationException + ? $"DDS error: {ex.Message}" + : $"Unexpected error: {ex.Message}"; + private static IReadOnlyList LoadDeals( string arg, TextReader? stdin) { From aee0d04e4bfa1f1095a7e55a72dc4efed51dff57 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 12:45:17 +0200 Subject: [PATCH 33/35] Include declaring seat when successive .NET par contracts differ. Match the C++/Python fix for #324 so thomas1-style W/E sacrifices keep both seats in FormatParLine instead of dropping the second seat. Co-authored-by: Cursor --- .../DdTableForDealLibTests.cs | 21 +++++++++++++++++-- dotnet/DdTableForDeal/DdTableForDealLib.cs | 4 +++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs index d13e7565..7a857395 100644 --- a/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs +++ b/dotnet/DdTableForDeal.Tests/DdTableForDealLibTests.cs @@ -232,7 +232,24 @@ public void MultipleSacrificesOnOneLine() } [Fact] - public void OmitsRepeatedDeclaringSideWhenSeatsDiffer() + public void IncludesDeclaringSideWhenSeatsDiffer() + { + // thomas1-style: W can sacrifice in hearts, E in clubs — both seats matter. + var sides = new ParResultsMaster[2]; + sides[0].Score = 100; + sides[0].Number = 2; + sides[0].Contracts[0] = MakeContract(/*W*/ 3, 3, /*H*/ 2, 1, 0); + sides[0].Contracts[1] = MakeContract(/*E*/ 1, 3, /*C*/ 4, 1, 0); + sides[1].Score = -100; + sides[1].Number = 2; + sides[1].Contracts[0] = MakeContract(/*W*/ 3, 3, /*H*/ 2, 1, 0); + sides[1].Contracts[1] = MakeContract(/*E*/ 1, 3, /*C*/ 4, 1, 0); + + Assert.Equal("Par: W 3Hx, E 3Cx -1 -100", DdTableForDealLib.FormatParLine(sides)); + } + + [Fact] + public void IncludesSeatWhenSecondContractNarrowsDeclaringSide() { var sides = new ParResultsMaster[2]; sides[0].Score = 100; @@ -244,7 +261,7 @@ public void OmitsRepeatedDeclaringSideWhenSeatsDiffer() sides[1].Contracts[0] = MakeContract(/*EW*/ 5, 4, /*H*/ 2, 1, 0); sides[1].Contracts[1] = MakeContract(/*E*/ 1, 5, /*C*/ 4, 1, 0); - Assert.Equal("Par: EW 4Hx, 5Cx -1 -100", DdTableForDealLib.FormatParLine(sides)); + Assert.Equal("Par: EW 4Hx, E 5Cx -1 -100", DdTableForDealLib.FormatParLine(sides)); } [Fact] diff --git a/dotnet/DdTableForDeal/DdTableForDealLib.cs b/dotnet/DdTableForDeal/DdTableForDealLib.cs index 60bb55d8..d845a59b 100644 --- a/dotnet/DdTableForDeal/DdTableForDealLib.cs +++ b/dotnet/DdTableForDeal/DdTableForDealLib.cs @@ -191,7 +191,9 @@ public static bool LooksLikePath(string arg) var body = new StringBuilder(); for (int i = 0; i < chosen.Number; i++) { - string? piece = FormatContract(chosen.Contracts[i], includeSeats: i == 0); + bool includeSeats = + i == 0 || chosen.Contracts[i].Seats != chosen.Contracts[i - 1].Seats; + string? piece = FormatContract(chosen.Contracts[i], includeSeats); if (piece is null) return null; if (i > 0) From 905668e31d32fd2e3110c2f2037d2ab7860357a6 Mon Sep 17 00:00:00 2001 From: Adam Wildavsky Date: Sat, 15 Aug 2026 13:05:22 +0200 Subject: [PATCH 34/35] Let native-load failures bubble past ProcessDeal; fix macOS mktemp. Co-authored-by: Cursor --- .../DdTableForDeal.Tests/ProcessDealErrorTests.cs | 14 ++++++++++++++ dotnet/DdTableForDeal/DdTableForDealApp.cs | 15 +++++++++++++-- dotnet/DdTableForDeal/e2e.sh | 3 ++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs b/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs index a3ca7016..f840d178 100644 --- a/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs +++ b/dotnet/DdTableForDeal.Tests/ProcessDealErrorTests.cs @@ -24,4 +24,18 @@ public void FormatProcessDealFailure_OtherException_IsUnexpectedError() Assert.Equal("Unexpected error: simulated stdout failure", message); } + + [Theory] + [InlineData(typeof(DllNotFoundException), true)] + [InlineData(typeof(EntryPointNotFoundException), true)] + [InlineData(typeof(BadImageFormatException), true)] + [InlineData(typeof(InvalidOperationException), false)] + [InlineData(typeof(IOException), false)] + public void IsNativeLibraryLoadFailure_ClassifiesLoaderVsOther( + Type exceptionType, bool expected) + { + Exception ex = (Exception)Activator.CreateInstance(exceptionType, "simulated")!; + + Assert.Equal(expected, DdTableForDealApp.IsNativeLibraryLoadFailure(ex)); + } } diff --git a/dotnet/DdTableForDeal/DdTableForDealApp.cs b/dotnet/DdTableForDeal/DdTableForDealApp.cs index 6385221e..23e22f0e 100644 --- a/dotnet/DdTableForDeal/DdTableForDealApp.cs +++ b/dotnet/DdTableForDeal/DdTableForDealApp.cs @@ -71,8 +71,7 @@ or InvalidOperationException } } } - catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException - or BadImageFormatException) + catch (Exception ex) when (IsNativeLibraryLoadFailure(ex)) { stderr.WriteLine( "Failed to load native DDS library. Build //jni:dds_shared and set " @@ -127,11 +126,23 @@ private static bool ProcessDeal( } catch (Exception ex) { + // Let loader failures reach Run's remediation message instead of + // being misreported as a per-deal unexpected error. + if (IsNativeLibraryLoadFailure(ex)) + throw; + stderr.WriteLine(FormatProcessDealFailure(ex)); return false; } } + /// + /// True for exceptions that mean the native DDS shared library failed to load + /// or resolve entry points (handled by with setup guidance). + /// + public static bool IsNativeLibraryLoadFailure(Exception ex) => + ex is DllNotFoundException or EntryPointNotFoundException or BadImageFormatException; + /// /// Maps ProcessDeal exceptions to CLI stderr lines: DDS return-code failures /// surface as InvalidOperationException; everything else is unexpected. diff --git a/dotnet/DdTableForDeal/e2e.sh b/dotnet/DdTableForDeal/e2e.sh index f4cf1a01..25614a2c 100755 --- a/dotnet/DdTableForDeal/e2e.sh +++ b/dotnet/DdTableForDeal/e2e.sh @@ -50,7 +50,8 @@ run_and_check() { echo "==> ${label}" local err_file out rc - err_file="$(mktemp)" + # BSD mktemp (macOS) requires a template; GNU mktemp accepts one too. + err_file="$(mktemp "${TMPDIR:-/tmp}/dd_table_for_deal.XXXXXX")" set +e out="$(dotnet run --project "$PROJECT" -- "$@" 2>"$err_file")" rc=$? From 9c452c353c1addbc8b441f015f8e1f7f136506b6 Mon Sep 17 00:00:00 2001 From: Martin Nygren Date: Sat, 15 Aug 2026 13:10:15 +0100 Subject: [PATCH 35/35] Clarifies the bindings overview. --- specs/dds-public-api.md | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/specs/dds-public-api.md b/specs/dds-public-api.md index 325f0157..bb6bf9f0 100644 --- a/specs/dds-public-api.md +++ b/specs/dds-public-api.md @@ -42,15 +42,24 @@ capability defines what crosses the boundary and promises to stay stable. configure/resize/clear, both resets, and the logging passthroughs. `SolverConfig` is decomposed into scalar arguments and `TTKind` crosses as an `int`, so no struct is passed by value. -- **Bindings pick different layers.** Java/FFM and .NET both bind the shim - (`dds_c_*`) plus the flat `dll.h` API — see - [jni-ffm-binding](jni-ffm-binding.md) and [dotnet-binding](dotnet-binding.md). - .NET reaches the shim through `EntryPoint` on its P/Invokes - (`dotnet/DDS_Core/Native/DdsNative.cs`), keeping its managed method names. - Python wraps the C++ API via pybind11 ([python-binding](python-binding.md)), - not the C shim. There is no shipped ctypes binding. The shim header is C-ABI - but not C-includable (it pulls in `dll.h`, which uses C++ trailing-return - syntax) — bind to compiled symbols or parse in C++ mode (jextract). +- **Bindings pick different layers.** The shim header + [`dds_c_api.h`](../library/src/api/dds_c_api.h) is C-ABI but not + C-includable (it pulls in `dll.h`, which uses C++ trailing-return syntax), + so each binding takes its own approach to it. + - **Python.** Wraps the C++ API via pybind11 + ([python-binding](python-binding.md)), not the C shim. There is no + shipped ctypes binding. + - **Java/FFM.** Binds the shim (`dds_c_*`) plus `GetDDSInfo` from the flat + `dll.h` API — see [jni-ffm-binding](jni-ffm-binding.md). Bindings are + hand-written straight to the compiled symbols rather than generated by + parsing the header; jextract could parse it in C++ mode instead, but + isn't used today since it ships only as non-hermetic early-access + binaries (see the header comment in `Dds.java`). + - **.NET.** Binds the shim (`dds_c_*`) plus most of the flat `dll.h` API + directly (`SolveBoard`, `CalcDDtable`, `Par`, `Analyse*`, …) — see + [dotnet-binding](dotnet-binding.md). Reaches the shim through + `EntryPoint` on its P/Invokes (`dotnet/DDS_Core/Native/DdsNative.cs`), + keeping its managed method names. - **Handles are single-threaded.** One `DDS_SOLVER_CTX` / `DDS_C_SOLVER_CTX` per thread; the handle owns per-context solver state and its transposition table. Create → use → destroy. The legacy flat API manages global/threaded state via