Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create stdbool flag #872

Closed
wants to merge 1 commit into from
Closed

Conversation

LEOYoon-Tsaw
Copy link
Member

@LEOYoon-Tsaw LEOYoon-Tsaw commented May 12, 2024

Pull request

Issue tracker

Fixes will automatically close the related issue

Fixes #

Feature

Use a flag STDBOOL to gate type definition for Bool. The current def to int causes compiler confusion in interoperation with Swift.
Ideally, Bool should use standard C bool instead of int. The use of a flag gate is for compatibility consideration.
Feel free to change the design if this is not the best practice

Unit test

  • Done

Manual test

  • Done

Code Review

  1. Unit and manual test pass
  2. GitHub Action CI pass
  3. At least one contributor reviews and votes
  4. Can be merged clean without conflicts
  5. PR will be merged by rebase upstream base

Additional Info

squirrel can be successfully setup to export this flag
LEOYoon-Tsaw/squirrel@39a51fc

@lotem
Copy link
Member

lotem commented May 13, 2024

Follow-up ideas:

  • new APIs use bool;
  • rename new APIs so they are never confused with the old APIs that use int for boolean;
  • guard the new APIs with a build flag;
  • the release build of the library on macOS provides both old and new APIs;
  • to use the new APIs, client code includes a separate header file with deferent macro definition for Bool.

@LEOYoon-Tsaw
Copy link
Member Author

Will be addressed by #877

@LEOYoon-Tsaw LEOYoon-Tsaw deleted the stdbool branch May 19, 2024 01:39
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