Handle DBL_DECIMAL_DIG inconsistency - #53
Merged
Merged
Conversation
There were two classes of warnings that I was getting a large number of with GCC's default warning settings: - many Mapping constructor calls were passing a `status` argument, but this is only valid in the internal API; the public API does not pass a status flag (just use astOk to check status), so this was resulting in unused printf argument warnings - other case was an empty string being passed as the `options` argument which does take a printf format string; GCC complains on empty printf format strings--the convention for this used elsewhere in the code is actually to pass a string containing just a single space
This addresses the question in PR Starlink#37: "One thing I have discovered is that the serialization tests fail by default because Claude decided that this was a C99-compatible repo and forces C99 standard. This is fine as such but we then run into a problem with AST_DBL_DIG being 18 in C99 mode (DBL_DIG [15] + 3) because DBL_DECIMAL_DIG does not formally exist in the C standard until C11. With C11 you get 17 digits and we get reproducible numbers in the output files for string comparisons." @dsberry's preferred fix (which I also agree is less disruptive) to set the fallback to 17 digits when compling for C99 resulting in consistency regardless of which C standard was compiled against; this fixes all the remaining test failures for me.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #53 +/- ##
=======================================
Coverage 54.88% 54.88%
=======================================
Files 372 372
Lines 116841 116841
Branches 35679 35679
=======================================
Hits 64126 64126
Misses 35601 35601
Partials 17114 17114 ☔ View full report in Codecov by Harness. |
Member
|
Thank you for doing this. |
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
2 participants
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.
This came up at #38: #38 (comment)
But was never handled. I agree that just setting the fallback
DBL_DECIMAL_DIGto be consistent between C99 and C11 is least disruptive, and this fixes all the test failure I was having on my machine where the compiler was defaulting to C99.Also fixed several build warnings that were coming from the Claude-generated tests (warnings that were enabled by default in GCC but maybe not in clang).