Don't promise particular values when float parsing fails.

This should address problems on Windows:
https://github.com/KhronosGroup/SPIRV-Tools/issues/104

diff --git a/include/util/hex_float.h b/include/util/hex_float.h
index 17434be..3ed4efc 100644 (file)
--- a/include/util/hex_float.h
+++ b/include/util/hex_float.h
@@ -772,11 +772,11 @@ inline bool RejectParseDueToLeadingSign(std::istream& is, bool negate_value,
 // If negate_value is true then the number may not have a leading minus or
 // plus, and if it successfully parses, then the number is negated before
 // being stored into the value parameter.
-// If the value cannot be correctly parsed, then set the fail bit on the
-// stream, and set the value to zero.
-// If the value overflows the target floating point type, then set the fail
-// bit on the stream and set the value to the nearest finite value for the
-// type, which can either be positive or negative.
+// If the value cannot be correctly parsed or overflows the target floating
+// point type, then set the fail bit on the stream.
+// TODO(dneto): Promise C++11 standard behavior in how the value is set in
+// the error case, but only after all target platforms implement it correctly.
+// In particular, the Microsoft C++ runtime appears to be out of spec.
 template <typename T, typename Traits>
 inline std::istream& ParseNormalFloat(std::istream& is, bool negate_value,
                                       HexFloat<T, Traits>& value) {
@@ -810,11 +810,11 @@ inline std::istream& ParseNormalFloat(std::istream& is, bool negate_value,
 // If negate_value is true then the number may not have a leading minus or
 // plus, and if it successfully parses, then the number is negated before
 // being stored into the value parameter.
-// If the value cannot be correctly parsed, then set the fail bit on the
-// stream, and set the value to zero.
-// If the value overflows the target floating point type, then set the fail
-// bit on the stream and set the value to the nearest finite value for the
-// type, which can either be positive or negative.
+// If the value cannot be correctly parsed or overflows the target floating
+// point type, then set the fail bit on the stream.
+// TODO(dneto): Promise C++11 standard behavior in how the value is set in
+// the error case, but only after all target platforms implement it correctly.
+// In particular, the Microsoft C++ runtime appears to be out of spec.
 template <>
 inline std::istream&
 ParseNormalFloat<FloatProxy<Float16>, HexFloatTraits<FloatProxy<Float16>>>(

diff --git a/test/HexFloat.cpp b/test/HexFloat.cpp
index f2257db..a2c5521 100644 (file)
--- a/test/HexFloat.cpp
+++ b/test/HexFloat.cpp
@@ -1030,9 +1030,11 @@ TEST_P(ParseNormalFloatTest, Samples) {
   EXPECT_NE(GetParam().expect_success, input.fail())
       << " literal: " << GetParam().literal
       << " negate: " << GetParam().negate_value;
-  EXPECT_THAT(parsed_value.value(), Eq(GetParam().expected_value.value()))
-      << " literal: " << GetParam().literal
-      << " negate: " << GetParam().negate_value;
+  if (GetParam().expect_success) {
+    EXPECT_THAT(parsed_value.value(), Eq(GetParam().expected_value.value()))
+        << " literal: " << GetParam().literal
+        << " negate: " << GetParam().negate_value;
+  }
 }
 
 // Returns a FloatParseCase with expected failure.
@@ -1095,9 +1097,11 @@ TEST_P(ParseNormalFloat16Test, Samples) {
   EXPECT_NE(GetParam().expect_success, input.fail())
       << " literal: " << GetParam().literal
       << " negate: " << GetParam().negate_value;
-  EXPECT_THAT(parsed_value.value(), Eq(GetParam().expected_value.value()))
-      << " literal: " << GetParam().literal
-      << " negate: " << GetParam().negate_value;
+  if (GetParam().expect_success) {
+    EXPECT_THAT(parsed_value.value(), Eq(GetParam().expected_value.value()))
+        << " literal: " << GetParam().literal
+        << " negate: " << GetParam().negate_value;
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -1142,7 +1146,9 @@ TEST_P(FloatProxyParseOverflowFloatTest, Sample) {
   HexFloat<FloatProxy<float>> value(0.0f);
   input >> value;
   EXPECT_NE(GetParam().expect_success, input.fail());
-  EXPECT_THAT(value.value().getAsFloat(), GetParam().expected_value);
+  if (GetParam().expect_success) {
+    EXPECT_THAT(value.value().getAsFloat(), GetParam().expected_value);
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -1167,7 +1173,9 @@ TEST_P(FloatProxyParseOverflowDoubleTest, Sample) {
   HexFloat<FloatProxy<double>> value(0.0);
   input >> value;
   EXPECT_NE(GetParam().expect_success, input.fail());
-  EXPECT_THAT(value.value().getAsFloat(), Eq(GetParam().expected_value));
+  if (GetParam().expect_success) {
+    EXPECT_THAT(value.value().getAsFloat(), Eq(GetParam().expected_value));
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -1193,8 +1201,10 @@ TEST_P(FloatProxyParseOverflowFloat16Test, Sample) {
   input >> value;
   EXPECT_NE(GetParam().expect_success, input.fail()) << " literal: "
                                                      << GetParam().input;
-  EXPECT_THAT(value.value().data(), Eq(GetParam().expected_value))
-      << " literal: " << GetParam().input;
+  if (GetParam().expect_success) {
+    EXPECT_THAT(value.value().data(), Eq(GetParam().expected_value))
+        << " literal: " << GetParam().input;
+  }
 }
 
 INSTANTIATE_TEST_CASE_P(