Skip to content

feat: complete API and unit tests for all types but Solver#40

Merged
abdoo8080 merged 13 commits intoabdoo8080:mainfrom
anzenlang:tm_api
Apr 7, 2026
Merged

feat: complete API and unit tests for all types but Solver#40
abdoo8080 merged 13 commits intoabdoo8080:mainfrom
anzenlang:tm_api

Conversation

@AdrienChampion
Copy link
Copy Markdown
Collaborator

  • lift all functions from the C++ API for all types but Solver, only exception is Term.toStringValue
  • lift all unit tests but Solver's, except Term.toStringValue tests

This PR does add that many functions to the API, as all types but Solver were already close to being complete. The main contribution is the unit tests, in particular for TermManager which has a lot more tests now.

Copy link
Copy Markdown
Owner

@abdoo8080 abdoo8080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I left some comments., but they can be addressed in a separate PR!

Comment on lines +216 to +217
-- -- no dedicated parser error
-- test![TestApiBlackInputParser, parserErrors]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can add them as new constructors to our cvc5.Error type?

Comment on lines +25 to +26
-- -- skipped as the lean API does not allow retrieving the solver
-- test![TestApiBlackInputParser, getSolver]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for not supporting this? If the reason is me, then we can go ahead and support it!

Comment on lines +28 to +29
-- -- skipped as the lean API does not allow retrieving the symbol manager
-- test![TestApiBlackInputParser, getSymbolManager]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto!

Comment on lines +124 to +125
-- -- tests `InputParser.setStreamInput`, which the lean API does not have
-- test![TestApiBlackInputParser, nextCommand]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add support for InputParser.setStreamInput using handles?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I assume you meant IO.FS.Stream and not IO.FS.Handle.)

I agree this should be investigated, but it seems a bit tricky to me, tricky enough to require some discussion. Unless we think this is an urgent feature I would postpone it for a later PR

@abdoo8080 abdoo8080 merged commit 80cf7b6 into abdoo8080:main Apr 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants