Skip to content

base: Fix double truncation in DynamicStringWriter and FixedStringWriter#5438

Open
backes wants to merge 3 commits intogoogle:mainfrom
backes:dev/clemensb/fix-double-truncation
Open

base: Fix double truncation in DynamicStringWriter and FixedStringWriter#5438
backes wants to merge 3 commits intogoogle:mainfrom
backes:dev/clemensb/fix-double-truncation

Conversation

@backes
Copy link
Copy Markdown

@backes backes commented Apr 13, 2026

When formatting large doubles like 1e100, %lf produces a fixed-point representation that can be very long. In DynamicStringWriter, this was truncated to 31 characters because of the 32-byte fixed buffer in StackString.

This CL changes the format from %lf to %.16g, which uses scientific notation for large and small exponents, ensuring it fits in the fixed buffer and preserving double precision.

This fixes an issue where V8's JsonIntegrationTest was failing when using Perfetto's JSON exporter (see https://crrev.com/c/7748468).

Bug: b/502067293

When formatting large doubles like 1e100, %lf produces a fixed-point
representation that can be very long. In DynamicStringWriter, this
was truncated to 31 characters because of the 32-byte fixed buffer
in StackString.

This CL changes the format from %lf to %.16g, which uses scientific
notation for large and small exponents, ensuring it fits in the
fixed buffer and preserving double precision.

This fixes an issue where V8's JsonIntegrationTest was failing when
using Perfetto's JSON exporter.

Bug: 382242434
@backes backes requested a review from a team as a code owner April 13, 2026 08:50
@backes
Copy link
Copy Markdown
Author

backes commented Apr 13, 2026

Thanks for the quick review!
I added two unitttests now for 1e100 (which was broken before).

Feel free to merge, I don't have permissions!

@LalitMaganti LalitMaganti enabled auto-merge (squash) April 13, 2026 14:32
@LalitMaganti LalitMaganti disabled auto-merge April 13, 2026 14:33
@LalitMaganti LalitMaganti enabled auto-merge (squash) April 13, 2026 14:33
@LalitMaganti
Copy link
Copy Markdown
Member

Seems like there's a UI test failure.

@backes
Copy link
Copy Markdown
Author

backes commented Apr 13, 2026

Oh, yes, we print a more precise value now.
Old:
image

New:
image

@backes
Copy link
Copy Markdown
Author

backes commented Apr 13, 2026

I see that the screenshot itself is not checked in, there's just a sha256 hash in ./test/data/ui-screenshots/table_viewer.test.ts/Go-to-slice/current-selection.png.sha256. How does that work? Where do I need to upload the actual new PNG?

auto-merge was automatically disabled April 13, 2026 14:52

Head branch was pushed to by a user without write access

@backes
Copy link
Copy Markdown
Author

backes commented Apr 13, 2026

Found ./tools/test_data upload. Worked flawlessly. Nice!

Should work now.

@backes
Copy link
Copy Markdown
Author

backes commented Apr 13, 2026

Merge conflict :/
And I have no idea how to generate that screenshot file locally...

@LalitMaganti
Copy link
Copy Markdown
Member

tools/run-integrationtests --rebaseline

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.

3 participants