This is an archive of the discontinued LLVM Phabricator instance.

[ADT] Add llvm::to_float
ClosedPublic

Authored by labath on Jun 22 2017, 10:37 AM.

Details

Summary

The function matches the interface of llvm::to_integer, but as we are
calling out to a C library function (implementing it ourselves seemed
way too complicated), I let it take a Twine argument, so we can avoid a
string copy at least in some cases.

I add a test and replace a couple of existing uses of strtod with this
function.

Event Timeline

labath created this revision.Jun 22 2017, 10:37 AM
zturner accepted this revision.Jun 22 2017, 10:40 AM

We also have APFloat::convertFromString, but it looks like it doesn't easily support long double. So, I guess this seems fine to me, but maybe wait a little longer and see if anyone else has suggestions

This revision is now accepted and ready to land.Jun 22 2017, 10:40 AM

it looks like it doesn't easily support long double

APFloat::convertFromString works for every floating-point type supported by APFloat; clang uses it to parse floating-point literals. There isn't any convenient way to convert an APFloat to a long double, but that's mostly because there hasn't been any demand for it; the format of long double is platform-dependent.

it looks like it doesn't easily support long double

APFloat::convertFromString works for every floating-point type supported by APFloat; clang uses it to parse floating-point literals. There isn't any convenient way to convert an APFloat to a long double, but that's mostly because there hasn't been any demand for it; the format of long double is platform-dependent.

I don't actually need long double support right now -- I just added it because it was easy.

So the lack of long double would not be a show-stopper for using an APFloat-based solution. However, the convertFromString seems to assume that the input string has already been checked for correctness, as it is full of asserts. This means it's impossible to use it as a drop in replacement for the "parse me this string I got from the user" type of functionality, which is what the existing users of strtod do, and what I intended to use it for. That could of course be fixed, but given that we don't need the fancy features of APFloat here, and strtod is readily available everywhere, I am not sure if it's really worth it.

This revision was automatically updated to reflect the committed changes.