Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(112)

Issue 2044643005: Add strtod test cases (Closed)

Created:
4 years, 6 months ago by scottmg
Modified:
4 years, 6 months ago
Reviewers:
Nico
CC:
chromium-reviews, jshin+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add strtod test cases Adds test cases from http://www.exploringbinary.com/how-strtod-works-and-sometimes-doesnt/. These currently fail on (see ps#1): - linux_chromium_gn_chromeos_rel - linux_chromium_asan_rel_ng - linux_chromium_chromeos_ozone_rel_ng - linux_chromium_rel_ng - cast_shell_linux but despite history appear to be fine on Windows (as of VS2015). As they were a bit of fiddling to get up and running, and we don't appear to have any similar tests for dmg_fp, add these tests so that we can hopefully drop dmg_fp in the future once std::strtod works more consistently everywhere. https://crbug.com/95729 suggests another solution where it'd be good to have these tests. BUG=593512, 615142, 616062, 588726, 95729 Committed: https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be Cr-Commit-Position: refs/heads/master@{#399948}

Patch Set 1 #

Patch Set 2 : don't change to std #

Total comments: 5

Patch Set 3 : use hexfloat instead #

Patch Set 4 : raw fp as uint64 #

Total comments: 2

Patch Set 5 : bit_cast #

Patch Set 6 : bit_cast for real #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -0 lines) Patch
M base/strings/string_number_conversions_unittest.cc View 1 2 3 4 5 2 chunks +51 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (14 generated)
scottmg
4 years, 6 months ago (2016-06-08 00:39:37 UTC) #11
Nico
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc#newcode808 base/strings/string_number_conversions_unittest.cc:808: // http://www.exploringbinary.com/converting-floating-point-numbers-to-binary-strings-in-c/ that doesn't say anything about license or ...
4 years, 6 months ago (2016-06-08 00:45:09 UTC) #12
Nico
(ps: hooray for doing this!)
4 years, 6 months ago (2016-06-08 00:45:35 UTC) #13
scottmg
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc#newcode808 base/strings/string_number_conversions_unittest.cc:808: // http://www.exploringbinary.com/converting-floating-point-numbers-to-binary-strings-in-c/ On 2016/06/08 00:45:09, Nico wrote: > that ...
4 years, 6 months ago (2016-06-08 02:28:18 UTC) #14
scottmg
https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc#newcode864 base/strings/string_number_conversions_unittest.cc:864: "100000101111001101100111011000101011101110110000110100.0"}, On 2016/06/08 02:28:18, scottmg wrote: > On 2016/06/08 ...
4 years, 6 months ago (2016-06-08 02:36:04 UTC) #15
scottmg
On 2016/06/08 02:36:04, scottmg wrote: > https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc > File base/strings/string_number_conversions_unittest.cc (right): > > https://codereview.chromium.org/2044643005/diff/20001/base/strings/string_number_conversions_unittest.cc#newcode864 > ...
4 years, 6 months ago (2016-06-08 02:40:30 UTC) #16
scottmg
Updated to just compare the raw double representation (no point in going back to a ...
4 years, 6 months ago (2016-06-14 20:46:43 UTC) #17
Nico
lgtm with one line changed. It's kind of impressive that string->float conversions are deterministic through ...
4 years, 6 months ago (2016-06-15 09:06:59 UTC) #18
scottmg
> It's kind of impressive that string->float conversions are deterministic through > all bits on ...
4 years, 6 months ago (2016-06-15 16:52:37 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2044643005/100001
4 years, 6 months ago (2016-06-15 17:04:40 UTC) #22
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 6 months ago (2016-06-15 17:52:59 UTC) #24
commit-bot: I haz the power
CQ bit was unchecked
4 years, 6 months ago (2016-06-15 17:53:01 UTC) #25
commit-bot: I haz the power
4 years, 6 months ago (2016-06-15 17:54:46 UTC) #27
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/8d7634d843a6b981d4b9bebe8f94412b6bccc3be
Cr-Commit-Position: refs/heads/master@{#399948}

Powered by Google App Engine
This is Rietveld 408576698