Skip to content

Address dotnet issues identified by Cursor #182

@tameware

Description

@tameware

Cursor spotted these for me, but I've never used dotnet. Best to let someone who has, and can test locally, address. - AW

DDS_Core .NET wrapper — issues review

Summary

Review of dotnet/DDS_Core found one runtime bug (missing native export), several API gaps in SolverContext, and a set of cleanup / consistency items. The CalcDdTable ThrowIfError / nameof bug (nameof(SolveBoard) instead of nameof(CalcDdTable)) has been fixed on branch dotnet-issues.


Critical

1. SolverContext.CalcPar — missing C export in dds_native

Files: dotnet/DDS_Core/Native/DdsNative.cs, library/src/api/dds_api.hpp, library/src/api/dds_api.cpp

DdsNative declares a P/Invoke for calc_par:

public static extern int calc_par(
    SolverContextHandle ctx,
    in DdTableDeal table_deal,
    int vulnerable,
    out DdTableResults table_results,
    out ParResults par_results);

SolverContext.CalcPar calls this export. However, dds_api.hpp / dds_api.cpp only export context management, dds_solve_board, calc_ddtable, and calc_ddtable_pbn. There is no extern "C" wrapper for calc_par.

Impact: Calling SolverContext.CalcPar(...) is expected to throw EntryPointNotFoundException at runtime.

Why it was missed: DDS_Core_Demo exercises legacy dds.CalcPar(...) but does not call ctx.CalcPar(...).

Suggested fix: Add to dds_api.hpp / dds_api.cpp, mirroring calc_ddtable:

EXTERN_C DLLEXPORT auto calc_par(
    DDS_SOLVER_CTX ctx,
    const DdTableDeal& table_deal,
    int vulnerable,
    DdTableResults* table_results,
    ParResults* par_results) -> int;

Delegate to the existing C++ calc_par(SolverContext&, ...) in library/src/calc_par.cpp.


API gaps (SolverContext vs legacy DDS)

The modern API is documented as the recommended path, but SolverContext is incomplete compared to legacy DDS and docs/dotnet_interface.md.

Missing on SolverContext Available on legacy DDS
SolveBoard(DealPBN, ...) Yes
CalcPar(DdTableDealPBN, ...) Yes
calc_par_from_table (par from pre-computed DD table) Python/C++ have it; .NET has only a commented-out stub in DdsNative.cs

Workarounds today:

  • PBN solve / par: use legacy DDS methods (marked [Obsolete]).
  • Par from existing table: use legacy DDS.Par(...), or CalcDdTable + Par manually.

Note: calc_par_from_table is C++-only today (not in dll.h). Exposing it to .NET would need a new extern "C" export in dds_api, similar to calc_par.


Bugs / inconsistencies (lower severity)

2. Inconsistent CalcPar parameter order (public C# API)

File: dotnet/DDS_Core/DDS.cs

  • Binary overload: CalcPar(DdTableDeal, int vulnerable, out table, out par)
  • PBN overload: CalcPar(DdTableDealPBN, out table, int vulnerable, out par)

Native call order matches the C API in both cases, but the public C# signatures are inconsistent and easy to misuse.

3. Broken XML doc comment

File: dotnet/DDS_Core/DDS.csCalcPar(DdTableDealPBN, ...)

///    /// <param name="vulnerable">

Double /// breaks the doc comment for the vulnerable parameter.

4. Demo calls FreeMemory() inside SolverContext flow

File: dotnet/DDS_Core_Demo/Program.csdoSolveBoardV3

Calls dds.FreeMemory() in a loop while using SolverContext. The PBN doCalcDdTableV3 path correctly uses ctx.ResetForSolve() instead. Mixing legacy global memory management with context-based API is misleading and may cause subtle issues.

5. SolveAllBoards (binary) does not zero out parameter before native call

File: dotnet/DDS_Core/DDS.cs

The PBN overload sets solved = default before the native call; the binary overload does not. Minor — only relevant on failure paths.


Code hygiene

6. Spurious using directives

Likely copy-paste leftovers; safe to remove:

File Unused import
DDS.cs using static System.Formats.Asn1.AsnWriter;
DdsNative.cs using static System.Net.WebRequestMethods;
SolverContext.cs System.Linq, System.Threading.Tasks, System.Collections.Generic
SolverContextHandle.cs System.Linq, System.Threading.Tasks, System.Reflection.Metadata, etc.
DDS_Core_Demo/Program.cs System.Transactions, MediaTypeNames, SafeHandles, etc.

7. Duplicated ThrowIfError implementation

Identical #if DEBUG helper exists in both DDS.cs and SolverContext.cs. Consider a shared internal helper to avoid drift.

8. ThrowIfError only throws in DEBUG builds

Documented in docs/dotnet_interface.md, but worth noting: Release builds return error codes without throwing. Callers must check return values in Release; DEBUG may mask missing checks.


Fixed (for reference)

CalcDdTable wrong error message name

File: dotnet/DDS_Core/DataModel/SolverContext.cs

The DdTableDeal overload of CalcDdTable passed nameof(SolveBoard) to ThrowIfError instead of nameof(CalcDdTable), producing misleading DEBUG exception messages. Fixed on dotnet-issues (commit b40e194).


Suggested test plan

  • Add dds_api export for calc_par and verify SolverContext.CalcPar runs without EntryPointNotFoundException.
  • Add demo or unit test calling ctx.CalcPar(...) (currently untested).
  • Decide scope for PBN / calc_par_from_table on SolverContext and align docs/dotnet_interface.md.
  • Remove spurious using directives.
  • Fix demo doSolveBoardV3 to stop calling dds.FreeMemory() in the context path.

Environment

  • Library: DDS_Core (.NET 8+)
  • Native dependency: dds_native (P/Invoke, CallingConvention.Cdecl)
  • Branch reviewed: dotnet-issues (includes CalcDdTable nameof fix)

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions