You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by GitBox <gi...@apache.org> on 2022/01/14 21:26:41 UTC
[GitHub] [thrift] rgumpertz commented on a change in pull request #2502: THRIFT-5496: Generate sufficient digits after the decimal point to n…
rgumpertz commented on a change in pull request #2502:
URL: https://github.com/apache/thrift/pull/2502#discussion_r785179647
##########
File path: compiler/cpp/src/thrift/generate/t_erl_generator.cc
##########
@@ -389,27 +390,36 @@ void t_erl_generator::close_generator() {
const std::string emit_double_as_string(const double value) {
std::stringstream double_output_stream;
- // sets the maximum precision: http://en.cppreference.com/w/cpp/io/manip/setprecision
- // sets the output format to fixed: http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
- double_output_stream << std::setprecision(std::numeric_limits<double>::digits10 + 1);
-
- #ifdef _MSC_VER
- // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
- // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
- // c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
- // and
- // http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
- #if _MSC_VER >= MSC_2015_VER
- double_output_stream << std::fixed;
- #else
- // note that if this function is called from the erlang generator and the MSVC compiler is older than 2015,
- // the double literal must be output in the scientific format. There can be some cases where the
- // mantissa of the output does not have fractionals, which is illegal in Erlang.
- // example => 10000000000000000.0 being output as 1e+16
- double_output_stream << std::scientific;
- #endif
+
+ #if defined(_MSC_VER) && _MSC_VER < MSC_2015_VER
+ // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
+ // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
+ // c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
+ // and
+ // http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
+ // note that if this function is called from the erlang generator and the MSVC compiler is older than 2015,
+ // the double literal must be output in the scientific format. There can be some cases where the
+ // mantissa of the output does not have fractionals, which is illegal in Erlang.
+ // example => 10000000000000000.0 being output as 1e+16
+ double_output_stream << std::scientific << std::setprecision(std::numeric_limits<double>::digits10 + 1);
#else
- double_output_stream << std::fixed;
+ //question from Thrift@Rick.Gumpertz.com: Does anybody know why we avoid scientific notation here?
+ double_output_stream << std::fixed;
+ #if 0 //maintain compatibility with previous versions of emit_double_as_string that could generate extra trailing digits
+ const int min_precision = std::numeric_limits<double>::digits10 + 1;
+ #else //don't add unneeded 0's
+ const int min_precision = 1;
+ #endif
+ if (std::isfinite(value) && value != 0.0) {
+ double_output_stream <<
+ std::setprecision(std::max(min_precision,
+ std::numeric_limits<double>::digits10 + 1
+ - static_cast<int>(floor(log10(abs(value))))
+ )
+ );
+ } else {
+ double_output_stream << std::setprecision(min_precision);
+ }
Review comment:
That's fine with me
##########
File path: compiler/cpp/src/thrift/generate/t_erl_generator.cc
##########
@@ -389,27 +390,36 @@ void t_erl_generator::close_generator() {
const std::string emit_double_as_string(const double value) {
std::stringstream double_output_stream;
- // sets the maximum precision: http://en.cppreference.com/w/cpp/io/manip/setprecision
- // sets the output format to fixed: http://en.cppreference.com/w/cpp/io/manip/fixed (not in scientific notation)
- double_output_stream << std::setprecision(std::numeric_limits<double>::digits10 + 1);
-
- #ifdef _MSC_VER
- // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
- // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
- // c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
- // and
- // http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
- #if _MSC_VER >= MSC_2015_VER
- double_output_stream << std::fixed;
- #else
- // note that if this function is called from the erlang generator and the MSVC compiler is older than 2015,
- // the double literal must be output in the scientific format. There can be some cases where the
- // mantissa of the output does not have fractionals, which is illegal in Erlang.
- // example => 10000000000000000.0 being output as 1e+16
- double_output_stream << std::scientific;
- #endif
+
+ #if defined(_MSC_VER) && _MSC_VER < MSC_2015_VER
+ // strtod is broken in MSVC compilers older than 2015, so std::fixed fails to format a double literal.
+ // more details: https://blogs.msdn.microsoft.com/vcblog/2014/06/18/
+ // c-runtime-crt-features-fixes-and-breaking-changes-in-visual-studio-14-ctp1/
+ // and
+ // http://www.exploringbinary.com/visual-c-plus-plus-strtod-still-broken/
+ // note that if this function is called from the erlang generator and the MSVC compiler is older than 2015,
+ // the double literal must be output in the scientific format. There can be some cases where the
+ // mantissa of the output does not have fractionals, which is illegal in Erlang.
+ // example => 10000000000000000.0 being output as 1e+16
+ double_output_stream << std::scientific << std::setprecision(std::numeric_limits<double>::digits10 + 1);
#else
- double_output_stream << std::fixed;
+ //question from Thrift@Rick.Gumpertz.com: Does anybody know why we avoid scientific notation here?
+ double_output_stream << std::fixed;
+ #if 0 //maintain compatibility with previous versions of emit_double_as_string that could generate extra trailing digits
+ const int min_precision = std::numeric_limits<double>::digits10 + 1;
+ #else //don't add unneeded 0's
+ const int min_precision = 1;
+ #endif
+ if (std::isfinite(value) && value != 0.0) {
+ double_output_stream <<
+ std::setprecision(std::max(min_precision,
+ std::numeric_limits<double>::digits10 + 1
Review comment:
That would make the constant have wider scope when it is intrinsically limited to this code. I would leave it where it is. Nevertheless, I'll defer to you if you consider it important.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org