Keep radix for integer literals in generated bindings #3237
Conversation
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
miikkas
force-pushed
the
keep-radix
branch
3 times, most recently
from
8f8d206 to
7054e0f
Compare
|
I have now included additional testing to the PR. Regarding the 13 failing existing tests: It seems that the differences are due to bindgen now generating, as intended, Rust literals using hexadecimal representation, while the values themselves have correctly stayed the same. Should I fix the expected code accordingly? |
Use the different radix literals for integers that were used in the original C and C++ header files, now that it is supported. The new values were confirmed by hand to be the same as the original ones - they are just represented using the Rust equivalent of the original form.
|
The test I added was changed to use C++14 instead of C23, as it seems that the latter is not available for clang 9.0, which IIUC is used in the CI. I have changed the expected side of the previously failing existing tests. Those changes are currently included in a separate commit in case that makes review less cumbersome. I will squash it later as instructed in the contribution guidelines. |
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'm not sure if the solution is too "hacky", as I don't have experience with the codebase. Testing is still lacking – I'll be sure to still work on that.
Fixes #3236.