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/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 roundTrip = [] (const char* str) {
+    return convertToString(convertToDoubleFromString(str), 0, 3).c_str();
+  };
+  EXPECT_STREQ("+Inf", roundTrip("+Inf"));
+  EXPECT_STREQ("+Inf", roundTrip("INFINITY"));
+  EXPECT_STREQ("+Inf", roundTrip("inf"));
+  EXPECT_STREQ("-Inf", roundTrip("-Inf"));
+  EXPECT_STREQ("-Inf", roundTrip("-INFINITY"));
+  EXPECT_STREQ("-Inf", roundTrip("-inf"));
+  EXPECT_STREQ("NaN", roundTrip("NaN"));
+  EXPECT_STREQ("NaN", roundTrip("nan"));
+  EXPECT_STREQ("NaN", roundTrip("-NaN"));
+  EXPECT_STREQ("NaN", roundTrip("-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());