facebook / facebook/folly

folly::to<int_32t>(double) throws loss of precision exceptions

Open
#491 2 comments 2 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

Here is a brief, reproducible code fragment example, written using `gtest` and `glog` syntax. It passes.

```
TEST_F(Test, TestCrashFollyConv) {
double y = 1.1;
int32_t x = (int32_t)y;
ASSERT_TRUE(x == 1);
x = floor(y);
ASSERT_TRUE(x == 1);
bool folly_threw = false;
try {
x = folly::to(y);
} catch (std::exception& e) {
LOG(ERROR) << "CAUGHT ERROR IN folly::to(double): " << e.what();
folly_threw = true;
}
ASSERT_TRUE(folly_threw);
}
```

Informally experimenting it appears an arbitrary number of `double` values fail to cast, raising this same error.

Of course there are multiple workarounds, such as `floor()` and `ceil()` in `cmath` specifically for this purpose. But the docs for `Conv` specifically claim "converts anything to anything" so I'm reporting it.

Taking a look at the code, float to int comparisons use lookup tables per digit that throw out of range exceptions if a position in the table is reached that doesn't match an expected position that would yield a digit. This is possibly the culprit, as under some failure scenarios I saw `std::range_error` raised, and in some saw the error message `to<>: loss of precision when converting 1.1 to type i`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.