From ae6368c98566a29c73e3aa94d1742ccbaf3b6dce Mon Sep 17 00:00:00 2001 From: vitaut Date: Tue, 26 Jan 2016 07:13:34 -0800 Subject: [PATCH 1/4] Add a comment to clarify why convert to unsigned --- format.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/format.cc b/format.cc index fbf932fc..2aa38bc0 100644 --- a/format.cc +++ b/format.cc @@ -328,6 +328,9 @@ class ArgConverter : public fmt::internal::ArgVisitor, void> { } else { if (is_signed) { arg_.type = Arg::LONG_LONG; + // Convert value to unsigned type before converting to long long for + // consistency with glibc's printf even though in general it's UB: + // std::printf("%lld", -42); // prints "4294967254" arg_.long_long_value = static_cast::Type>(value); } else { From 7ee287d3d9e340737f3ad66292e7bc60a53b3053 Mon Sep 17 00:00:00 2001 From: vitaut Date: Wed, 27 Jan 2016 07:03:19 -0800 Subject: [PATCH 2/4] Sign extend arguments of smaller types passed to %ll? (#265) --- format.cc | 7 +++---- test/printf-test.cc | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/format.cc b/format.cc index 2aa38bc0..995e5727 100644 --- a/format.cc +++ b/format.cc @@ -328,11 +328,10 @@ class ArgConverter : public fmt::internal::ArgVisitor, void> { } else { if (is_signed) { arg_.type = Arg::LONG_LONG; - // Convert value to unsigned type before converting to long long for - // consistency with glibc's printf even though in general it's UB: + // glibc's printf doesn't sign extend arguments of smaller types: // std::printf("%lld", -42); // prints "4294967254" - arg_.long_long_value = - static_cast::Type>(value); + // but we don't have to do the same because it's a UB. + arg_.long_long_value = value; } else { arg_.type = Arg::ULONG_LONG; arg_.ulong_long_value = diff --git a/test/printf-test.cc b/test/printf-test.cc index 12b50cdf..5607aa6a 100644 --- a/test/printf-test.cc +++ b/test/printf-test.cc @@ -306,8 +306,8 @@ void TestLength(const char *length_spec, U value) { } using fmt::internal::MakeUnsigned; if (sizeof(U) <= sizeof(int) && sizeof(int) < sizeof(T)) { - signed_value = unsigned_value = - static_cast::Type>(value); + signed_value = value; + unsigned_value = static_cast::Type>(value); } else { signed_value = static_cast::Type>(value); unsigned_value = static_cast::Type>(value); From 95c0fb507556906c2d3f6c9ea934034678f8310c Mon Sep 17 00:00:00 2001 From: vitaut Date: Fri, 29 Jan 2016 16:29:46 -0800 Subject: [PATCH 3/4] Add a "C" numeric locale --- posix.h | 52 +++++++++++++++++++++++++++++++++++++++++++++- test/posix-test.cc | 9 ++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/posix.h b/posix.h index 88bcb4f5..3f02f388 100644 --- a/posix.h +++ b/posix.h @@ -34,8 +34,10 @@ #endif #include -#include // for O_RDONLY +#include // for O_RDONLY +#include // for locale_t #include +#include // for strtod_l #include @@ -331,6 +333,54 @@ class File { // Returns the memory page size. long getpagesize(); + +#if defined(LC_NUMERIC_MASK) || defined(_MSC_VER) + +// A "C" numeric locale. +class Locale { + private: +# ifdef _MSC_VER + typedef _locale_t locale_t; + + enum { LC_NUMERIC_MASK = LC_NUMERIC }; + + static locale_t newlocale(int category_mask, const char *locale, locale_t) { + return _create_locale(category_mask, locale); + } + + static void freelocale(locale_t locale) { + _free_locale(locale); + } + + static double strtod_l(const char *nptr, char **endptr, _locale_t locale) { + return _strtod_l(nptr, endptr, locale); + } +# endif + + locale_t locale_; + + FMT_DISALLOW_COPY_AND_ASSIGN(Locale); + + public: + Locale() : locale_(newlocale(LC_NUMERIC_MASK, "C", NULL)) { + if (!locale_) + throw fmt::SystemError(errno, "cannot create locale"); + } + ~Locale() { freelocale(locale_); } + + locale_t get() const { return locale_; } + + // Converts string to floating-point number and advances str past the end + // of the parsed input. + double strtod(const char *&str) const { + char *end = 0; + double result = strtod_l(str, &end, locale_); + str = end; + return result; + } +}; + +#endif // LC_NUMERIC } // namespace fmt #if !FMT_USE_RVALUE_REFERENCES diff --git a/test/posix-test.cc b/test/posix-test.cc index 1484481f..ac42538b 100644 --- a/test/posix-test.cc +++ b/test/posix-test.cc @@ -387,3 +387,12 @@ TEST(FileTest, FdopenError) { EXPECT_SYSTEM_ERROR_NOASSERT( f.fdopen("r"), EBADF, "cannot associate stream with file descriptor"); } + +#ifdef LC_NUMERIC_MASK +TEST(LocaleTest, Strtod) { + fmt::Locale locale; + const char *start = "4.2", *ptr = start; + EXPECT_EQ(4.2, locale.strtod(ptr)); + EXPECT_EQ(start + 3, ptr); +} +#endif From 4952e79e4588eb24a6f4e469f44aa4e527ce2d49 Mon Sep 17 00:00:00 2001 From: vitaut Date: Sat, 30 Jan 2016 09:20:43 -0800 Subject: [PATCH 4/4] Document that floating-point formatting is locale-dependent. --- doc/syntax.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/syntax.rst b/doc/syntax.rst index 6ec33d93..8e8d2e46 100644 --- a/doc/syntax.rst +++ b/doc/syntax.rst @@ -262,6 +262,8 @@ The available presentation types for floating-point values are: | none | The same as ``'g'``. | +---------+----------------------------------------------------------+ +Floating-point formatting is locale-dependent. + .. ifconfig:: False +---------+----------------------------------------------------------+