Repository navigation
Write the minimum signed integer correctly in CSVWriter - #336
Conversation
to_string for signed integers negated the value before casting it to size_t, which overflows for INT_MIN: the negation wraps back to INT_MIN and the cast sign-extends it, so INT_MIN was written as -18446744071562067968. Negate in unsigned arithmetic instead.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #336 +/- ##
=======================================
Coverage 91.54% 91.54%
=======================================
Files 55 55
Lines 3595 3595
Branches 1068 1068
=======================================
Hits 3291 3291
Misses 115 115
Partials 189 189
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 258f4d1e8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| REQUIRE(csv::internals::to_string(-0.666) == "-0.66600"); | ||
| } | ||
|
|
||
| SECTION("Negative Integers") { |
There was a problem hiding this comment.
This new Negative Integers section is an edge-case regression, but it is inserted in the opening numeric-converter case ahead of all the mainline writer tests that occupy the rest of the file. Make it a final regression test case so this file preserves the repository's required mainline-first/edge-cases-last organization.
AGENTS.md reference: tests/AGENTS.md:L104-L110
Useful? React with 👍 / 👎.
2 participants
CSVWriterwritesINT_MINas a 20-digit number:internals::to_stringfor signed integers doesto_string((size_t)(value * -1)). For the minimum value the negation overflows (UBSan: "negation of -2147483648 cannot be represented in type 'int'" atcsv_writer.hpp:132); in practice it wraps back toINT_MIN, and the cast tosize_tsign-extends it.long longhappens to print right, but UBSan flagsLLONG_MINtoo. This negates in unsigned arithmetic:"-" + to_string(0ULL - static_cast<unsigned long long>(value)).Regression tests at the end of
tests/test_write_csv.cppcover-1,INT_MIN,LLONG_MIN, zero, andlong long::max(). A publicmake_csv_writertest writes the extrema in a mixed tuple and checks the complete CSV output. Expected values use standard-library formatting for portability.Validation: the writer suite passes locally with MSVC on x64 C++20 and x86 C++11. The x86 validation invocation suppresses existing dependency alignment and narrowing warnings; repository build settings are unchanged. Single-include and Emscripten validation are left to CI.
Written with AI assistance (Claude); I have reviewed the change.