[PATCH] D57321: Fix LexFloatLiteral Lexing

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 21 12:53:29 PST 2019


efriedma added a comment.

So I guess overall, there are three fixes here:

1. Make AsmLexer::LexDigit handle floats without a decimal point more consistently.
2. Make AsmLexer::LexFloatLiteral print an error for floats which are apparently missing an "e".
3. Make APFloat::convertFromString use binutils-compatible exponent parsing.

Is that right?



================
Comment at: llvm/lib/MC/MCParser/AsmLexer.cpp:77
+  // literal as the upstream client cannot parse all invalid
+  // forms and crashes. (e.g. 1e8e8)
+  if ((*CurPtr == 'e' || *CurPtr == 'E')) {
----------------
Maybe update this comment?


================
Comment at: llvm/lib/MC/MCParser/AsmLexer.cpp:153
+    if (!IsIdentifierChar(*CurPtr, AllowAtInIdentifier) ||
+        *CurPtr == 'e' || *CurPtr == 'E')
       return LexFloatLiteral();
----------------
This change doesn't do anything?


================
Comment at: llvm/test/MC/AsmParser/floating-literals.s:60
+# CHECK-ERROR: Invalid sign in float literal
+.double 2.+1
 
----------------
We should probably have testcases for 1E1, 1e1e1, and 1e-1, since those don't work correctly without this patch.


================
Comment at: llvm/unittests/ADT/APFloatTest.cpp:1196
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "1.0e-"), "Exponent has no digits");
-}
-
----------------
We should probably keep these testcases, just change them to check for the new behavior (using ASSERT_EQ).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57321/new/

https://reviews.llvm.org/D57321





More information about the llvm-commits mailing list