Fix APFloat from string conversion for Inf

[yrouban]

Summary

The method IEEEFloat::convertFromStringSpecials() does not recognize the "+Inf" and "-Inf" strings but these strings are printed for the double Infinities by the IEEEFloat::toString().

This patch adds the "+Inf" and "-Inf" strings to the list of recognized patterns in IEEEFloat::convertFromStringSpecials().

[yrouban]

What can I do to make a progress with this patch?

[skatkov]

LGTM.

[yrouban]

In D38030#892957, @skatkov wrote:

LGTM.

Could you please land this patch?

[skatkov]

Will land it in a 24-hours if there is no objections from others.

[FlameTop]

I'm afraid this has broken Windows builds (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/12860/steps/test/logs/stdio).

The test fails with

[ RUN ] APFloatTest.fromToStringSpecials

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(856): error: Expected: "+Inf"

To be equal to: roundTrip("+Inf")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(857): error: Expected: "+Inf"

To be equal to: roundTrip("INFINITY")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(858): error: Expected: "+Inf"

To be equal to: roundTrip("inf")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(859): error: Expected: "-Inf"

To be equal to: roundTrip("-Inf")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(860): error: Expected: "-Inf"

To be equal to: roundTrip("-INFINITY")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(861): error: Expected: "-Inf"

To be equal to: roundTrip("-inf")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(862): error: Expected: "NaN"

To be equal to: roundTrip("NaN")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(863): error: Expected: "NaN"

To be equal to: roundTrip("nan")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(864): error: Expected: "NaN"

To be equal to: roundTrip("-NaN")

Which is: ""

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\unittests\ADT\APFloatTest.cpp(865): error: Expected: "NaN"

To be equal to: roundTrip("-nan")

Which is: ""

[ FAILED ] APFloatTest.fromToStringSpecials (24 ms)

[mkazantsev]

I have reverted the patch as rL316158.

[skatkov]

Thank you guys, Yevgeny, please take a look.

[yrouban]

In D38030#901911, @skatkov wrote:

Thank you guys, Yevgeny, please take a look.

OK, I will.

[yrouban]

fixed bug in APFloatTest.cpp to not return reference to a temporary string.

[anna]

lgtm.

just for information of folks following along - we had caught this as a heap-use-after-free error using an ASAN build. I think this may have manifested as the Windows failure that caused the revert originally.

[yrouban]

Committers, could you please try landing the patch?

[skatkov]

I'll try

llvm/trunk/lib/Support/APFloat.cpp

/* We can finally safely perform the max-exponent check. */
 
     delete [] decSignificand;
   }
 
   return fs;
 }
 
 bool IEEEFloat::convertFromStringSpecials(StringRef str) {
-  if (str.equals("inf") || str.equals("INFINITY")) {
+  if (str.equals("inf") || str.equals("INFINITY") || str.equals("+Inf")) {
     makeInf(false);
     return true;
   }
 
-  if (str.equals("-inf") || str.equals("-INFINITY")) {
+  if (str.equals("-inf") || str.equals("-INFINITY") || str.equals("-Inf")) {
     makeInf(true);
     return true;
   }
 
   if (str.equals("nan") || str.equals("NaN")) {
     makeNaN(false, false);
     return true;
   }

llvm/trunk/lib/Support/StringRef.cpp

bool StringRef::getAsInteger(unsigned Radix, APInt &Result) const {
   return false;
 }
 
 bool StringRef::getAsDouble(double &Result, bool AllowInexact) const {
   APFloat F(0.0);
   APFloat::opStatus Status =
       F.convertFromString(*this, APFloat::rmNearestTiesToEven);
   if (Status != APFloat::opOK) {
-    if (!AllowInexact || Status != APFloat::opInexact)
+    if (!AllowInexact || !(Status & APFloat::opInexact))
       return true;
   }
 
   Result = F.convertToDouble();
   return false;
 }
 
 // Implementation of StringRef hashing.
 hash_code llvm::hash_value(StringRef S) {
   return hash_combine_range(S.begin(), S.end());
 }

llvm/trunk/unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, fromDecimalString) {
   EXPECT_TRUE(APFloat(APFloat::IEEEdouble(), "99e99999").isInfinity());
   EXPECT_TRUE(APFloat(APFloat::IEEEdouble(), "-99e99999").isInfinity());
   EXPECT_TRUE(APFloat(APFloat::IEEEdouble(), "1e-99999").isPosZero());
   EXPECT_TRUE(APFloat(APFloat::IEEEdouble(), "-1e-99999").isNegZero());
 
   EXPECT_EQ(2.71828, convertToDoubleFromString("2.71828"));
 }
 
+TEST(APFloatTest, fromToStringSpecials) {
+  auto expects = [] (const char *first, const char *second) {
+    std::string roundtrip = convertToString(convertToDoubleFromString(second), 0, 3);
+    EXPECT_STREQ(first, roundtrip.c_str());
+  };
+  expects("+Inf", "+Inf");
+  expects("+Inf", "INFINITY");
+  expects("+Inf", "inf");
+  expects("-Inf", "-Inf");
+  expects("-Inf", "-INFINITY");
+  expects("-Inf", "-inf");
+  expects("NaN", "NaN");
+  expects("NaN", "nan");
+  expects("NaN", "-NaN");
+  expects("NaN", "-nan");
+}
+
 TEST(APFloatTest, fromHexadecimalString) {
   EXPECT_EQ( 1.0, APFloat(APFloat::IEEEdouble(),  "0x1p0").convertToDouble());
   EXPECT_EQ(+1.0, APFloat(APFloat::IEEEdouble(), "+0x1p0").convertToDouble());
   EXPECT_EQ(-1.0, APFloat(APFloat::IEEEdouble(), "-0x1p0").convertToDouble());
 
   EXPECT_EQ( 1.0, APFloat(APFloat::IEEEdouble(),  "0x1p+0").convertToDouble());
   EXPECT_EQ(+1.0, APFloat(APFloat::IEEEdouble(), "+0x1p+0").convertToDouble());
   EXPECT_EQ(-1.0, APFloat(APFloat::IEEEdouble(), "-0x1p+0").convertToDouble());

llvm/trunk/unittests/ADT/StringRefTest.cpp

struct GetDoubleStrings {
   const char *Str;
   bool AllowInexact;
   bool ShouldFail;
   double D;
 } DoubleStrings[] = {{"0", false, false, 0.0},
                      {"0.0", false, false, 0.0},
                      {"-0.0", false, false, -0.0},
                      {"123.45", false, true, 123.45},
-                     {"123.45", true, false, 123.45}};
+                     {"123.45", true, false, 123.45},
+                     {"1.8e308", true, false, std::numeric_limits<double>::infinity()},
+                     {"1.8e308", false, true, std::numeric_limits<double>::infinity()},
+                     {"0x0.0000000000001P-1023", false, true, 0.0},
+                     {"0x0.0000000000001P-1023", true, false, 0.0},
+                    };
 
 TEST(StringRefTest, getAsDouble) {
   for (const auto &Entry : DoubleStrings) {
     double Result;
     StringRef S(Entry.Str);
     EXPECT_EQ(Entry.ShouldFail, S.getAsDouble(Result, Entry.AllowInexact));
     if (!Entry.ShouldFail) {
       EXPECT_EQ(Result, Entry.D);