Remove thousands separator format specifier (')

eliaskosunen · Jan 10, 2024 · c086798 · c086798
1 parent e227010
commit c086798
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 59 deletions.
diff --git a/include/scn/detail/format_string_parser.h b/include/scn/detail/format_string_parser.h
@@ -110,13 +110,11 @@ namespace scn {
             unsigned arbitrary_base : 6;
             unsigned align : 2;
             bool localized : 1;
-            bool thsep : 1;
 
             constexpr basic_format_specs()
                 : arbitrary_base{0},
                   align{static_cast<unsigned>(align_type::none)},
-                  localized{false},
-                  thsep{false}
+                  localized{false}
             {
             }
 
@@ -252,11 +250,6 @@ namespace scn {
                 m_specs.regexp_flags = flags;
             }
 
-            constexpr void on_thsep()
-            {
-                m_specs.thsep = true;
-            }
-
             // Intentionally not constexpr
             void on_error(const char* msg)
             {
@@ -797,15 +790,6 @@ namespace scn {
                 return begin;
             }
 
-            if (*begin == CharT{'\''}) {
-                handler.on_thsep();
-                ++begin;
-            }
-            if (SCN_UNLIKELY(begin == end)) {
-                handler.on_error("Unexpected end of format string");
-                return begin;
-            }
-
             if (begin != end && *begin != CharT{'}'}) {
                 do_presentation();
             }
@@ -1019,38 +1003,11 @@ namespace scn {
 
                 Handler::on_localized();
             }
-            constexpr void on_thsep()
-            {
-                const auto cat = get_category_for_arg_type(m_arg_type);
-                if (cat != arg_type_category::integer &&
-                    cat != arg_type_category::unsigned_integer &&
-                    cat != arg_type_category::floating) {
-                    SCN_UNLIKELY_ATTR
-                    return this->on_error(
-                        "' specifier (for a thousands separator) can only "
-                        "be "
-                        "used with arguments of integer or floating-point "
-                        "types");
-                }
-
-                Handler::on_thsep();
-            }
 
         private:
             arg_type m_arg_type;
         };
 
-        template <typename CharT, typename Handler>
-        constexpr void check_disallow_thsep(
-            const basic_format_specs<CharT>& specs,
-            Handler&& handler)
-        {
-            if (SCN_UNLIKELY(specs.thsep)) {
-                return handler.on_error(
-                    "' specifier not allowed for this type");
-            }
-        }
-
         template <typename CharT, typename Handler>
         constexpr void check_int_type_specs(
             const basic_format_specs<CharT>& specs,
@@ -1079,7 +1036,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type > presentation_type::int_hex ||
                 specs.type == presentation_type::int_arbitrary_base) {
                 SCN_UNLIKELY_ATTR
@@ -1093,7 +1049,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type != presentation_type::none &&
                 specs.type != presentation_type::character) {
                 SCN_UNLIKELY_ATTR
@@ -1121,7 +1076,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type == presentation_type::none ||
                 specs.type == presentation_type::string ||
                 specs.type == presentation_type::string_set ||
@@ -1146,7 +1100,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type != presentation_type::none &&
                 specs.type != presentation_type::pointer) {
                 SCN_UNLIKELY_ATTR
@@ -1159,7 +1112,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type != presentation_type::none &&
                 specs.type != presentation_type::string &&
                 specs.type != presentation_type::int_generic &&
@@ -1178,7 +1130,6 @@ namespace scn {
             const basic_format_specs<CharT>& specs,
             Handler&& handler)
         {
-            check_disallow_thsep(specs, handler);
             if (specs.type == presentation_type::regex ||
                 specs.type == presentation_type::regex_escaped) {
                 return;

diff --git a/src/scn/impl/reader/float_reader.h b/src/scn/impl/reader/float_reader.h
@@ -678,7 +678,7 @@ namespace scn {
                 const detail::basic_format_specs<CharT>& specs)
             {
                 unsigned options{};
-                if (specs.thsep) {
+                if (specs.localized) {
                     options |= float_reader_base::allow_thsep;
                 }
 

diff --git a/src/scn/impl/reader/integer_reader.h b/src/scn/impl/reader/integer_reader.h
@@ -369,7 +369,7 @@ namespace scn {
                     return std::next(prefix_result.iterator);
                 }
 
-                if (SCN_LIKELY(!specs.thsep)) {
+                if (SCN_LIKELY(!specs.localized)) {
                     SCN_TRY(after_digits_it,
                             parse_integer_digits_without_thsep(
                                 ranges::subrange{prefix_result.iterator,

diff --git a/tests/unittests/format_string_parser_test.cpp b/tests/unittests/format_string_parser_test.cpp
@@ -29,7 +29,6 @@ TEST(FormatStringParserTest, DefaultConstructedSpecs)
     EXPECT_EQ(specs.align,
               static_cast<unsigned>(scn::detail::align_type::none));
     EXPECT_EQ(specs.localized, false);
-    EXPECT_EQ(specs.thsep, false);
 }
 
 TEST(FormatStringParserTest, ParsePresentationType)
@@ -55,7 +54,7 @@ namespace scn {
         {
             return a.width == b.width && a.fill == b.fill && a.type == b.type &&
                    a.arbitrary_base == b.arbitrary_base && a.align == b.align &&
-                   a.localized == b.localized && a.thsep == b.thsep;
+                   a.localized == b.localized;
         }
     }  // namespace detail
 

diff --git a/tests/unittests/impl_tests/float_reader_test.cpp b/tests/unittests/impl_tests/float_reader_test.cpp
@@ -1025,7 +1025,6 @@ struct thsep_test_state {
                  new numpunct_with_comma_thsep<CharT>{std::move(grouping)}),
           locref(stdloc)
     {
-        specs.thsep = true;
     }
 
     scn::detail::basic_format_specs<CharT> specs{};
@@ -1035,6 +1034,10 @@ struct thsep_test_state {
 
 TYPED_TEST(FloatValueReaderTest, ThousandsSeparators)
 {
+    if constexpr (!TestFixture::is_localized) {
+        return SUCCEED() << "This test requires a localized reader";
+    }
+
     auto state = thsep_test_state<typename TestFixture::char_type>{"\3"};
 
     auto [a, _, val] = this->simple_success_specs_and_locale_test(
@@ -1045,6 +1048,10 @@ TYPED_TEST(FloatValueReaderTest, ThousandsSeparators)
 
 TYPED_TEST(FloatValueReaderTest, ThousandsSeparatorsWithInvalidGrouping)
 {
+    if constexpr (!TestFixture::is_localized) {
+        return SUCCEED() << "This test requires a localized reader";
+    }
+
     auto state = thsep_test_state<typename TestFixture::char_type>{"\3"};
 
     auto [result, val] = this->simple_specs_and_locale_test(

diff --git a/tests/unittests/impl_tests/integer_reader_test.h b/tests/unittests/impl_tests/integer_reader_test.h
@@ -487,7 +487,7 @@ class IntValueReaderTest : public testing::Test {
         SCN_GCC_PUSH
         SCN_GCC_IGNORE("-Wconversion")
         specs.arbitrary_base = arb_base;
-        specs.thsep = thsep;
+        specs.localized = thsep;
         SCN_GCC_POP
         return specs;
     }
@@ -798,7 +798,6 @@ struct thsep_test_state {
                  new numpunct_with_comma_thsep<CharT>{std::move(grouping)}),
           locref(stdloc)
     {
-        specs.thsep = true;
     }
 
     scn::detail::basic_format_specs<CharT> specs{};
@@ -811,6 +810,9 @@ TYPED_TEST_P(IntValueReaderTest, ThousandsSeparators)
     if constexpr (!TestFixture::has_thsep_value()) {
         return SUCCEED() << "Type too small to hold '123,456'";
     }
+    if constexpr (!TestFixture::is_localized) {
+        return SUCCEED() << "This test requires a localized reader";
+    }
 
     auto state = thsep_test_state<typename TestFixture::char_type>{"\3"};
 
@@ -825,6 +827,9 @@ TYPED_TEST_P(IntValueReaderTest, ThousandsSeparatorsWithInvalidGrouping)
     if constexpr (!TestFixture::has_thsep_value()) {
         return SUCCEED() << "Type too small to hold '123,456'";
     }
+    if constexpr (!TestFixture::is_localized) {
+        return SUCCEED() << "This test requires a localized reader";
+    }
 
     auto state = thsep_test_state<typename TestFixture::char_type>{"\3"};
 

diff --git a/tests/unittests/integer_test.cpp b/tests/unittests/integer_test.cpp
@@ -158,15 +158,15 @@ TEST(IntegerTest, LongInput)
 TEST(IntegerTest, WonkyInputWithThsep)
 {
     std::string_view input = "-0x,)27614,)24t14741";
-    auto result = scn::scan<int>(input, "{:'}");
+    auto result = scn::scan<int>(input, "{:L}");
     ASSERT_TRUE(result);
     EXPECT_EQ(result->begin(), input.begin() + 2);
     EXPECT_EQ(result->value(), 0);
 }
 TEST(IntegerTest, WonkyInputWithThsep2)
 {
     std::string_view input = "-0b,28";
-    auto result = scn::scan<int>(input, "{:'}");
+    auto result = scn::scan<int>(input, "{:L}");
     ASSERT_TRUE(result);
     EXPECT_EQ(result->begin(), input.begin() + 2);
     EXPECT_EQ(result->value(), 0);