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

[hfinkel]

unittests/AsmParser/AsmParserTest.cpp

90 ↗

(On Diff #42286)

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 ↗

(On Diff #42286)

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.

llvm/trunk/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;
 }