subject:"\[PATCH\] D69770\: Add recoverable string parsing errors to APFloat"

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-19 Thread Ehud Katz via Phabricator via cfe-commits

ekatz added a comment.

Ping


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

https://reviews.llvm.org/D69770



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-11 Thread Ehud Katz via Phabricator via cfe-commits

ekatz added a comment.

Ping


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

https://reviews.llvm.org/D69770



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-05 Thread Ehud Katz via Phabricator via cfe-commits

ekatz updated this revision to Diff 227917.
ekatz added a comment.

Fixed requested changes.


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

https://reviews.llvm.org/D69770

Files:
  clang/lib/Lex/LiteralSupport.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/APFloat.cpp
  llvm/lib/Support/StringRef.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
  llvm/unittests/ADT/APFloatTest.cpp

Index: llvm/unittests/ADT/APFloatTest.cpp
===
--- llvm/unittests/ADT/APFloatTest.cpp
+++ llvm/unittests/ADT/APFloatTest.cpp
@@ -10,8 +10,8 @@
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -20,9 +20,17 @@
 
 using namespace llvm;
 
-static double convertToDoubleFromString(const char *Str) {
+static std::string convertToErrorFromString(StringRef Str) {
   llvm::APFloat F(0.0);
-  F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven);
+  auto ErrOrStatus =
+  F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven);
+  EXPECT_TRUE(!ErrOrStatus);
+  return toString(ErrOrStatus.takeError());
+}
+
+static double convertToDoubleFromString(StringRef Str) {
+  llvm::APFloat F(0.0);
+  EXPECT_FALSE(!F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven));
   return F.convertToDouble();
 }
 
@@ -1147,172 +1155,172 @@
   EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0.0f).convertToDouble(), "Float semantics are not IEEEdouble");
   EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0.0 ).convertToFloat(),  "Float semantics are not IEEEsingle");
 }
+#endif
+#endif
 
-TEST(APFloatTest, StringDecimalDeath) {
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(),  ""), "Invalid string length");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "+"), "String has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "-"), "String has no digits");
+TEST(APFloatTest, StringDecimalError) {
+  EXPECT_EQ("Invalid string length", convertToErrorFromString(""));
+  EXPECT_EQ("String has no digits", convertToErrorFromString("+"));
+  EXPECT_EQ("String has no digits", convertToErrorFromString("-"));
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("\0", 1)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1\0", 2)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1" "\0" "2", 3)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1" "\0" "2e1", 5)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e\0", 3)), "Invalid character in exponent");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e1\0", 4)), "Invalid character in exponent");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e1" "\0" "2", 5)), "Invalid character in exponent");
+  EXPECT_EQ("Invalid character in significand", convertToErrorFromString(StringRef("\0", 1)));
+  EXPECT_EQ("Invalid character in significand", convertToErrorFromString(StringRef("1\0", 2)));
+  EXPECT_EQ("Invalid character in significand", convertToErrorFromString(StringRef("1" "\0" "2", 3)));
+  EXPECT_EQ("Invalid character in significand", convertToErrorFromString(StringRef("1" "\0" "2e1", 5)));
+  EXPECT_EQ("Invalid character in exponent", convertToErrorFromString(StringRef("1e\0", 3)));
+  EXPECT_EQ("Invalid character in exponent", convertToErrorFromString(StringRef("1e1\0", 4)));
+  EXPECT_EQ("Invalid character in exponent", convertToErrorFromString(StringRef("1e1" "\0" "2", 5)));
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "1.0f"), "Invalid character in significand");
+  EXPECT_EQ("Invalid character in significand", convertToErrorFromString("1.0f"));
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), ".."), "String contains multiple dots");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "..0"), "String contains multiple dots");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "1.0.0"), "String contains multiple dots");
+  EXPECT_EQ("String contains multiple dots", convertToErrorFromString(".."));
+  EXPECT_EQ("String contains multiple dots", convertToErrorFromString("..0"));
+  EXPECT_EQ("String contains multiple dots", convertToErrorFromString("1.0.0"));
 }
 
-TEST(APFloatTest, StringDecimalSignificandDeath) {
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(),  "."), "Significand has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "+."), "Significand has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "-."), "Significand has no digits");
+TEST(APFloatTest, StringDecimalSignificandError) {
+  EXPECT_EQ("Significand has no digits", convertToErrorFromString( "."));
+

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-05 Thread Ehud Katz via Phabricator via cfe-commits

ekatz marked 2 inline comments as done.
ekatz added inline comments.



Comment at: llvm/lib/Support/APFloat.cpp:273
+  if (p != end)
+return createError("Invalid exponent in exponent");
 

arsenm wrote:
> Error message sounds like nonsense 
It is actually incorrect. There should not be an error returned, but 
`absExponent` should still be clamped.



Comment at: llvm/unittests/ADT/APFloatTest.cpp:1322
+  EXPECT_EQ(convertToErrorFromString("+0x1.1p-"), "Exponent has no digits");
+  EXPECT_EQ(convertToErrorFromString("-0x1.1p-"), "Exponent has no digits");
 }

arsenm wrote:
> It’s a gtestism that these operands should be swapped
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69770



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-03 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added inline comments.



Comment at: llvm/lib/Support/APFloat.cpp:273
+  if (p != end)
+return createError("Invalid exponent in exponent");
 

Error message sounds like nonsense 



Comment at: llvm/unittests/ADT/APFloatTest.cpp:1322
+  EXPECT_EQ(convertToErrorFromString("+0x1.1p-"), "Exponent has no digits");
+  EXPECT_EQ(convertToErrorFromString("-0x1.1p-"), "Exponent has no digits");
 }

It’s a gtestism that these operands should be swapped


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69770



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69770: Add recoverable string parsing errors to APFloat

2019-11-03 Thread Ehud Katz via Phabricator via cfe-commits

ekatz created this revision.
ekatz added reviewers: chandlerc, lattner, rsmith.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, hiraditya, 
nhaehnle, jvesely, arsenm.
Herald added projects: clang, LLVM.

Implementing the APFloat part in PR4745.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69770

Files:
  clang/lib/Lex/LiteralSupport.cpp
  llvm/include/llvm/ADT/APFloat.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/APFloat.cpp
  llvm/lib/Support/StringRef.cpp
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
  llvm/unittests/ADT/APFloatTest.cpp

Index: llvm/unittests/ADT/APFloatTest.cpp
===
--- llvm/unittests/ADT/APFloatTest.cpp
+++ llvm/unittests/ADT/APFloatTest.cpp
@@ -10,8 +10,8 @@
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -20,9 +20,17 @@
 
 using namespace llvm;
 
-static double convertToDoubleFromString(const char *Str) {
+static std::string convertToErrorFromString(StringRef Str) {
   llvm::APFloat F(0.0);
-  F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven);
+  auto ErrOrStatus =
+  F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven);
+  EXPECT_TRUE(!ErrOrStatus);
+  return toString(ErrOrStatus.takeError());
+}
+
+static double convertToDoubleFromString(StringRef Str) {
+  llvm::APFloat F(0.0);
+  EXPECT_FALSE(!F.convertFromString(Str, llvm::APFloat::rmNearestTiesToEven));
   return F.convertToDouble();
 }
 
@@ -1147,172 +1155,172 @@
   EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0.0f).convertToDouble(), "Float semantics are not IEEEdouble");
   EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0.0 ).convertToFloat(),  "Float semantics are not IEEEsingle");
 }
+#endif
+#endif
 
-TEST(APFloatTest, StringDecimalDeath) {
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(),  ""), "Invalid string length");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "+"), "String has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "-"), "String has no digits");
+TEST(APFloatTest, StringDecimalError) {
+  EXPECT_EQ(convertToErrorFromString(""), "Invalid string length");
+  EXPECT_EQ(convertToErrorFromString("+"), "String has no digits");
+  EXPECT_EQ(convertToErrorFromString("-"), "String has no digits");
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("\0", 1)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1\0", 2)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1" "\0" "2", 3)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1" "\0" "2e1", 5)), "Invalid character in significand");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e\0", 3)), "Invalid character in exponent");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e1\0", 4)), "Invalid character in exponent");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), StringRef("1e1" "\0" "2", 5)), "Invalid character in exponent");
+  EXPECT_EQ(convertToErrorFromString(StringRef("\0", 1)), "Invalid character in significand");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1\0", 2)), "Invalid character in significand");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1" "\0" "2", 3)), "Invalid character in significand");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1" "\0" "2e1", 5)), "Invalid character in significand");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1e\0", 3)), "Invalid character in exponent");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1e1\0", 4)), "Invalid character in exponent");
+  EXPECT_EQ(convertToErrorFromString(StringRef("1e1" "\0" "2", 5)), "Invalid character in exponent");
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "1.0f"), "Invalid character in significand");
+  EXPECT_EQ(convertToErrorFromString("1.0f"), "Invalid character in significand");
 
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), ".."), "String contains multiple dots");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "..0"), "String contains multiple dots");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "1.0.0"), "String contains multiple dots");
+  EXPECT_EQ(convertToErrorFromString(".."), "String contains multiple dots");
+  EXPECT_EQ(convertToErrorFromString("..0"), "String contains multiple dots");
+  EXPECT_EQ(convertToErrorFromString("1.0.0"), "String contains multiple dots");
 }
 
-TEST(APFloatTest, StringDecimalSignificandDeath) {
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(),  "."), "Significand has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "+."), "Significand has no digits");
-  EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), "-."), "Significand has no

[PATCH] D69770: Add recoverable string parsing errors to APFloat

[PATCH] D69770: Add recoverable string parsing errors to APFloat

[PATCH] D69770: Add recoverable string parsing errors to APFloat

[PATCH] D69770: Add recoverable string parsing errors to APFloat

[PATCH] D69770: Add recoverable string parsing errors to APFloat

[PATCH] D69770: Add recoverable string parsing errors to APFloat

6 matches

Site Navigation

Mail list logo

Footer information