Fix of issue: Address dotnet issues identified by Cursor #182#184
Open
mortensp wants to merge 5 commits into
Open
Fix of issue: Address dotnet issues identified by Cursor #182#184mortensp wants to merge 5 commits into
mortensp wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses .NET interop and wrapper issues by standardizing exported native entry points (avoiding C++ name mangling), updating P/Invoke declarations to match, and cleaning up .NET XML docs and debug-only error throwing behavior.
Changes:
- Rename/standardize v3 native exports to
dds_*names and add a C-linkage wrapper export forcalc_par. - Update .NET P/Invoke and managed wrappers to call the new native entry points; make
ThrowIfErrordebug-only via[Conditional("DEBUG")]. - Fix XML docs and remove spurious/unused
usingdirectives in .NET projects.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| library/src/dds_api.cpp | Adds/renames exported wrapper functions (dds_calc_dd_table*, dds_calc_par) for stable interop symbols. |
| library/src/api/dds_api.hpp | Updates exported function declarations to dds_* names and adds dds_calc_par. |
| library/src/api/calc_par.hpp | Removes an inconsistent DLLEXPORT on the C++ overload declaration. |
| dotnet/DDS_Core/Native/DdsNative.cs | Updates DllImport declarations to call the new dds_* native exports; removes unused usings. |
| dotnet/DDS_Core/Helpers/SolverContextHandle.cs | Cleans unused usings; keeps SafeHandle-based lifetime management. |
| dotnet/DDS_Core/DDS.cs | Fixes XML doc issues, makes ThrowIfError debug-only, and adjusts parameter ordering in managed APIs. |
| dotnet/DDS_Core/DataModel/SolverContext.cs | Updates to new dds_* P/Invoke names and makes ThrowIfError debug-only. |
| dotnet/DDS_Core_Demo/Program.cs | Updates demo usage and adds a v3 CalcPar sample invocation. |
tameware
suggested changes
Jun 12, 2026
| /// </returns> | ||
| public int CalcPar( in DdTableDealPBN tableDealPBN | ||
| , out DdTableResults tableResults | ||
| , in int vulnerable |
Contributor
There was a problem hiding this comment.
I don't understand why the parameters to CalcPar are in a different order from those for CalcParPBN. If this is intentional and important I think it deserves a comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have made the following fixes:
Closes #182
Closes #184