[llvm] r306096 - [ADT] Add llvm::to_float
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 5 15:46:06 PDT 2017
Pavel Labath via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: labath
> Date: Fri Jun 23 07:55:02 2017
> New Revision: 306096
>
> URL: http://llvm.org/viewvc/llvm-project?rev=306096&view=rev
> Log:
> [ADT] Add llvm::to_float
>
> Summary:
> The function matches the interface of llvm::to_integer, but as we are
> calling out to a C library function, I let it take a Twine argument, so
> we can avoid a string copy at least in some cases.
Just like in my (unaddressed) review feedback about to_integer[1], I
feel pretty strongly that this isn't the right interface to implement.
It would be more useable and less error prone to provide an interface
returning optional instead:
template <typename FloatTy> Optional<FloatTy> to_float(const Twine &T)
[1]: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170612/461927.html
>
> I add a test and replace a couple of existing uses of strtod with this
> function.
>
> Reviewers: zturner
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D34518
>
> Modified:
> llvm/trunk/include/llvm/ADT/StringExtras.h
> llvm/trunk/lib/Support/CommandLine.cpp
> llvm/trunk/lib/Support/YAMLTraits.cpp
> llvm/trunk/unittests/ADT/StringExtrasTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/StringExtras.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringExtras.h?rev=306096&r1=306095&r2=306096&view=diff
> ==============================================================================
>
> --- llvm/trunk/include/llvm/ADT/StringExtras.h (original)
> +++ llvm/trunk/include/llvm/ADT/StringExtras.h Fri Jun 23 07:55:02 2017
> @@ -15,10 +15,12 @@
> #define LLVM_ADT_STRINGEXTRAS_H
>
> #include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/SmallString.h"
> #include "llvm/ADT/StringRef.h"
> #include <cassert>
> #include <cstddef>
> #include <cstdint>
> +#include <cstdlib>
> #include <cstring>
> #include <iterator>
> #include <string>
> @@ -129,6 +131,32 @@ template <typename N> bool to_integer(St
> return !S.getAsInteger(Base, Num);
> }
>
> +namespace detail {
> +template <typename N>
> +inline bool to_float(const Twine &T, N &Num, N (*StrTo)(const char *, char **)) {
> + SmallString<32> Storage;
> + StringRef S = T.toNullTerminatedStringRef(Storage);
> + char *End;
> + N Temp = StrTo(S.data(), &End);
> + if (*End != '\0')
> + return false;
> + Num = Temp;
> + return true;
> +}
> +}
> +
> +inline bool to_float(const Twine &T, float &Num) {
> + return detail::to_float(T, Num, std::strtof);
> +}
> +
> +inline bool to_float(const Twine &T, double &Num) {
> + return detail::to_float(T, Num, std::strtod);
> +}
> +
> +inline bool to_float(const Twine &T, long double &Num) {
> + return detail::to_float(T, Num, std::strtold);
> +}
> +
> static inline std::string utostr(uint64_t X, bool isNeg = false) {
> char Buffer[21];
> char *BufPtr = std::end(Buffer);
>
> Modified: llvm/trunk/lib/Support/CommandLine.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=306096&r1=306095&r2=306096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
> +++ llvm/trunk/lib/Support/CommandLine.cpp Fri Jun 23 07:55:02 2017
> @@ -24,6 +24,7 @@
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallPtrSet.h"
> #include "llvm/ADT/SmallString.h"
> +#include "llvm/ADT/StringExtras.h"
> #include "llvm/ADT/StringMap.h"
> #include "llvm/ADT/Twine.h"
> #include "llvm/Config/config.h"
> @@ -1522,13 +1523,9 @@ bool parser<unsigned long long>::parse(O
> // parser<double>/parser<float> implementation
> //
> static bool parseDouble(Option &O, StringRef Arg, double &Value) {
> - SmallString<32> TmpStr(Arg.begin(), Arg.end());
> - const char *ArgStart = TmpStr.c_str();
> - char *End;
> - Value = strtod(ArgStart, &End);
> - if (*End != 0)
> - return O.error("'" + Arg + "' value invalid for floating point argument!");
> - return false;
> + if (to_float(Arg, Value))
> + return false;
> + return O.error("'" + Arg + "' value invalid for floating point argument!");
> }
>
> bool parser<double>::parse(Option &O, StringRef ArgName, StringRef Arg,
>
> Modified: llvm/trunk/lib/Support/YAMLTraits.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLTraits.cpp?rev=306096&r1=306095&r2=306096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/YAMLTraits.cpp (original)
> +++ llvm/trunk/lib/Support/YAMLTraits.cpp Fri Jun 23 07:55:02 2017
> @@ -10,6 +10,7 @@
> #include "llvm/Support/YAMLTraits.h"
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallString.h"
> +#include "llvm/ADT/StringExtras.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/ADT/Twine.h"
> #include "llvm/Support/Casting.h"
> @@ -912,12 +913,9 @@ void ScalarTraits<double>::output(const
> }
>
> StringRef ScalarTraits<double>::input(StringRef Scalar, void *, double &Val) {
> - SmallString<32> buff(Scalar.begin(), Scalar.end());
> - char *end;
> - Val = strtod(buff.c_str(), &end);
> - if (*end != '\0')
> - return "invalid floating point number";
> - return StringRef();
> + if (to_float(Scalar, Val))
> + return StringRef();
> + return "invalid floating point number";
> }
>
> void ScalarTraits<float>::output(const float &Val, void *, raw_ostream &Out) {
> @@ -925,12 +923,9 @@ void ScalarTraits<float>::output(const f
> }
>
> StringRef ScalarTraits<float>::input(StringRef Scalar, void *, float &Val) {
> - SmallString<32> buff(Scalar.begin(), Scalar.end());
> - char *end;
> - Val = strtod(buff.c_str(), &end);
> - if (*end != '\0')
> - return "invalid floating point number";
> - return StringRef();
> + if (to_float(Scalar, Val))
> + return StringRef();
> + return "invalid floating point number";
> }
>
> void ScalarTraits<Hex8>::output(const Hex8 &Val, void *, raw_ostream &Out) {
>
> Modified: llvm/trunk/unittests/ADT/StringExtrasTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringExtrasTest.cpp?rev=306096&r1=306095&r2=306096&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/StringExtrasTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/StringExtrasTest.cpp Fri Jun 23 07:55:02 2017
> @@ -65,4 +65,22 @@ TEST(StringExtrasTest, ToAndFromHex) {
> EvenBytes.size());
> EXPECT_EQ(EvenStr, toHex(EvenData));
> EXPECT_EQ(EvenData, fromHex(EvenStr));
> -}
> \ No newline at end of file
> +}
> +
> +TEST(StringExtrasTest, to_float) {
> + float F;
> + EXPECT_TRUE(to_float("4.7", F));
> + EXPECT_FLOAT_EQ(4.7, F);
> +
> + double D;
> + EXPECT_TRUE(to_float("4.7", D));
> + EXPECT_DOUBLE_EQ(4.7, D);
> +
> + long double LD;
> + EXPECT_TRUE(to_float("4.7", LD));
> + EXPECT_DOUBLE_EQ(4.7, LD);
> +
> + EXPECT_FALSE(to_float("foo", F));
> + EXPECT_FALSE(to_float("7.4 foo", F));
> + EXPECT_FLOAT_EQ(4.7, F); // F should be unchanged
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list