emmenlau commented on a change in pull request #2502:
URL: https://github.com/apache/thrift/pull/2502#discussion_r785306192
##########
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 [email protected]: 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:
Yes, I can see that there is a tradeoff with the good scoping. The
reason for my suggestion is that `std::numeric_limits<double>::digits10 + 1` is
a rather "complex" term that is used in multiple places. It could add the
readability and ease future maintenance to assign it to a more expressive
variable, like `default_floating_precision` (is that what it is?). That also
makes the long bracketed term a bit shorter :-)
Its not strictly required, but it would help me read the code.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
