Fix PR #25788: parsing of floating-point constants on non-C locales

[hfinkel]

unittests/AsmParser/AsmParserTest.cpp

90

I don't think we can use this here because it is not thread safe (and gtest might run multiple tests in parallel).

I'm fine with this going in without a regression test if there's no reasonable way to construct one.

[pitrou]

unittests/AsmParser/AsmParserTest.cpp

90

Yes, it's the only way of testing I could think about. Unfortunately the Unix locale system isn't very flexible...

I'll strip the test code and re-submit if nobody else suggests an idea.

[pitrou]

Here is an updated patch without the locale-changing test.

[pitrou]

Just a ping. I'm not sure there's anything holding this patch?

[pitrou]

Ping again :-) This is bitting our users on a regular basis: https://github.com/numba/numba/issues/1656

[majnemer]

I think we'd want APFloat::convertFromDecimalString with rmNearestTiesToEven seeing as how we don't expect to see -nan and the like.

[pitrou]

I think we'd want APFloat::convertFromDecimalString with rmNearestTiesToEven seeing as how we don't expect to see -nan and the like.

I've looked at this and there are a couple issues:

• convertFromDecimalString() is private
• convertFromDecimalString() doesn't handle leading sign

Also, due to the lexing that occurs before calling APFloat() here, I don't think it can actually be called with -nan and friends. I'll upload an updated patch as it turns out the one here doesn't apply anymore.

[pitrou]

Updated against git master.

[majnemer]

LGTM

[pitrou]

Sorry for the ping, but this patch has been marked "accepted and ready to land", and I can't see it on master. Is anything needed on my part?

[meadori]

Do you have commit rights?

If not, then I will commit it for you today.

[pitrou]

Thanks, Meador! No, I don't.

lib/AsmParser/LLLexer.cpp

if (CurPtr[0] == 'e' || CurPtr[0] == 'E') {
     if (isdigit(static_cast<unsigned char>(CurPtr[1])) ||
         ((CurPtr[1] == '-' || CurPtr[1] == '+') &&
           isdigit(static_cast<unsigned char>(CurPtr[2])))) {
       CurPtr += 2;
       while (isdigit(static_cast<unsigned char>(CurPtr[0]))) ++CurPtr;
     }
   }
 
-  APFloatVal = APFloat(std::atof(TokStart));
+  APFloatVal = APFloat(APFloat::IEEEdouble,
+                       StringRef(TokStart, CurPtr - TokStart));
   return lltok::APFloat;
 }
 
 /// Lex a floating point constant starting with +.
 ///    FPConstant  [-+]?[0-9]+[.][0-9]*([eE][-+]?[0-9]+)?
 lltok::Kind LLLexer::LexPositive() {
   // If the letter after the negative is a number, this is probably not a
   // label.
if (CurPtr[0] == 'e' || CurPtr[0] == 'E') {
     if (isdigit(static_cast<unsigned char>(CurPtr[1])) ||
         ((CurPtr[1] == '-' || CurPtr[1] == '+') &&
         isdigit(static_cast<unsigned char>(CurPtr[2])))) {
       CurPtr += 2;
       while (isdigit(static_cast<unsigned char>(CurPtr[0]))) ++CurPtr;
     }
   }
 
-  APFloatVal = APFloat(std::atof(TokStart));
+  APFloatVal = APFloat(APFloat::IEEEdouble,
+                       StringRef(TokStart, CurPtr - TokStart));
   return lltok::APFloat;
 }

unittests/AsmParser/AsmParserTest.cpp

 #include "llvm/AsmParser/Parser.h"
 #include "llvm/AsmParser/SlotMapping.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
+#if !LLVM_ON_WIN32
+#include <locale.h>
+#endif
+
 using namespace llvm;
 
 namespace {
 
 TEST(AsmParserTest, NullTerminatedInput) {
   LLVMContext &Ctx = getGlobalContext();
   StringRef Source = "; Empty module \n";
   SMDiagnostic Error;
TEST(AsmParserTest, TypeAndConstantValueParsing) {
 
   const Value *V;
   V = parseConstantValue("double 3.5", Error, M);
   ASSERT_TRUE(V);
   EXPECT_TRUE(V->getType()->isDoubleTy());
   ASSERT_TRUE(isa<ConstantFP>(V));
   EXPECT_TRUE(cast<ConstantFP>(V)->isExactlyValue(3.5));
 
+  // https://llvm.org/bugs/show_bug.cgi?id=25788
+#if !LLVM_ON_WIN32
+  char *oldLoc = setlocale(LC_ALL, NULL);
+  if (setlocale(LC_ALL, "fr_FR.UTF-8")) {
+    V = parseConstantValue("double 1.25e+01", Error, M);
+    setlocale(LC_ALL, oldLoc);
+    ASSERT_TRUE(V);
+    EXPECT_TRUE(V->getType()->isDoubleTy());
+    ASSERT_TRUE(isa<ConstantFP>(V));
+    EXPECT_TRUE(cast<ConstantFP>(V)->isExactlyValue(12.5));
+  }
+#endif
+
   V = parseConstantValue("i32 42", Error, M);
   ASSERT_TRUE(V);
   EXPECT_TRUE(V->getType()->isIntegerTy());
   ASSERT_TRUE(isa<ConstantInt>(V));
   EXPECT_TRUE(cast<ConstantInt>(V)->equalsInt(42));
 
   V = parseConstantValue("<4 x i32> <i32 0, i32 1, i32 2, i32 3>", Error, M);
   ASSERT_TRUE(V);