diff mbox series

[committed] libstdc++: The Trouble with Tribbles

Message ID 20221206213420.232451-1-jwakely@redhat.com
State Committed
Commit 48e21e878b2c6cfc7797088a7393a735de75883c
Headers
Series [committed] libstdc++: The Trouble with Tribbles |

Commit Message

Jonathan Wakely Dec. 6, 2022, 9:34 p.m. UTC 
  Tested x86_64-linux. Pushed to trunk.

-- >8 --

Fix digit grouping for integers formatted with "{:#Lx}" which were
including the "0x" prefix in the grouped digits. This resulted in output
like "0,xff,fff" instead of "0xff,fff".

Also change std:::basic_format_parse_context to not throw for an arg-id
that is larger than the actual number of format arguments. I clarified
with Victor Zverovich that this is the intended behaviour for the
run-time format-string checks. An out-of-range arg-id should be
diagnosed at compile-time (as clarified by LWG 3825) but not run-time.
The formatting function will still throw at run-time when args.arg(id)
returns an empty basic_format_arg.

libstdc++-v3/ChangeLog:

	* include/std/format (basic_format_parse_context::next_arg_id):
	Only check arg-id is in range during constant evaluation.
	* testsuite/std/format/functions/format.cc: Check "{:#Lx}".
	* testsuite/std/format/parse_ctx.cc: Adjust expected results for
	format-strings using an out-of-range arg-id.
---
 libstdc++-v3/include/std/format               | 29 +++++++-----
 .../testsuite/std/format/functions/format.cc  |  4 ++
 .../testsuite/std/format/parse_ctx.cc         | 45 ++++++++-----------
 3 files changed, 40 insertions(+), 38 deletions(-)
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
index cb5ce40dece..6d6a770eb8c 100644
--- a/libstdc++-v3/include/std/format
+++ b/libstdc++-v3/include/std/format
@@ -221,7 +221,10 @@  namespace __format
 	if (_M_indexing == _Manual)
 	  __format::__conflicting_indexing_in_format_string();
 	_M_indexing = _Auto;
-	// if (std::is_constant_evaluated()) // XXX skip runtime check?
+
+	// _GLIBCXX_RESOLVE_LIB_DEFECTS
+	// 3825. Missing compile-time argument id check in next_arg_id
+	if (std::is_constant_evaluated())
 	  if (_M_next_arg_id == _M_num_args)
 	    __format::__invalid_arg_id_in_format_string();
 	return _M_next_arg_id++;
@@ -234,7 +237,7 @@  namespace __format
 	  __format::__conflicting_indexing_in_format_string();
 	_M_indexing = _Manual;
 
-	// if (std::is_constant_evaluated()) // XXX skip runtime check?
+	if (std::is_constant_evaluated())
 	  if (__id >= _M_num_args)
 	    __format::__invalid_arg_id_in_format_string();
       }
@@ -1167,15 +1170,18 @@  namespace __format
 		  string __grp = __np.grouping();
 		  if (!__grp.empty())
 		    {
-		      size_t __n = __str.size();
+		      size_t __n = __str.size() - __prefix_len;
 		      auto __p = (_CharT*)__builtin_alloca(2 * __n
-							     * sizeof(_CharT));
-		      auto __end = std::__add_grouping(__p,
+							     * sizeof(_CharT)
+							     + __prefix_len);
+		      auto __s = __str.data();
+		      char_traits<_CharT>::copy(__p, __s, __prefix_len);
+		      __s += __prefix_len;
+		      auto __end = std::__add_grouping(__p + __prefix_len,
 						       __np.thousands_sep(),
 						       __grp.data(),
 						       __grp.size(),
-						       __str.data(),
-						       __str.data() + __n);
+						       __s, __s + __n);
 		      __str = {__p, size_t(__end - __p)};
 		    }
 		}
@@ -3381,10 +3387,11 @@  namespace __format
       __ctx.advance_to(__format::__write(__ctx.out()));
     }
 
-  // TODO define __process_format_string which takes an object with callbacks
-  // can use that for initial constexpr parse of format string (with callbacks
-  // that have no side effects, just non-constant on error).
-
+  // Abstract base class defining an interface for scanning format strings.
+  // Scan the characters in a format string, dividing it up into strings of
+  // ordinary characters, escape sequences, and replacement fields.
+  // Call virtual functions for derived classes to parse format-specifiers
+  // or write formatted output.
   template<typename _CharT>
     struct _Scanner
     {
diff --git a/libstdc++-v3/testsuite/std/format/functions/format.cc b/libstdc++-v3/testsuite/std/format/functions/format.cc
index 8019fbdf712..7a155208a48 100644
--- a/libstdc++-v3/testsuite/std/format/functions/format.cc
+++ b/libstdc++-v3/testsuite/std/format/functions/format.cc
@@ -111,6 +111,10 @@  test_std_examples()
     string s3 = format("{:L}", 1234);
     VERIFY(s3 == "1,234");
 
+    // Test locale's "byte-and-a-half" grouping (Imperial word? tribble?).
+    string s4 = format("{:#Lx}", 0xfffff);
+    VERIFY(s4 == "0xff,fff");
+
     // Restore
     std::locale::global(std::locale::classic());
   }
diff --git a/libstdc++-v3/testsuite/std/format/parse_ctx.cc b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
index dd6ca68290b..069dfceced5 100644
--- a/libstdc++-v3/testsuite/std/format/parse_ctx.cc
+++ b/libstdc++-v3/testsuite/std/format/parse_ctx.cc
@@ -4,11 +4,11 @@ 
 #include <format>
 #include <testsuite_hooks.h>
 
-template<typename T, size_t N = 1, bool auto_indexing = true>
+template<typename T, bool auto_indexing = true>
 bool
 is_std_format_spec_for(std::string_view spec)
 {
-  std::format_parse_context pc(spec, N);
+  std::format_parse_context pc(spec);
   if (auto_indexing)
     (void) pc.next_arg_id();
   else
@@ -49,11 +49,9 @@  test_char()
   VERIFY( ! is_std_format_spec_for<char>("0") );
   VERIFY( ! is_std_format_spec_for<char>("00d") );
   VERIFY( is_std_format_spec_for<char>("01d") );
-  VERIFY( ! is_std_format_spec_for<char>("0{}d") );
+  VERIFY( is_std_format_spec_for<char>("0{}d") );
   VERIFY( ! is_std_format_spec_for<char>("0{1}d") );
-  VERIFY(( is_std_format_spec_for<char, 2>("0{}d") ));
-  VERIFY(( ! is_std_format_spec_for<char, 2>("0{1}d") ));
-  VERIFY(( is_std_format_spec_for<char, 2, false>("0{1}d") ));
+  VERIFY(( is_std_format_spec_for<char, false>("0{1}d") ));
   VERIFY( is_std_format_spec_for<char>("1") );
   VERIFY( ! is_std_format_spec_for<char>("-1") );
   VERIFY( is_std_format_spec_for<char>("-1d") ); // sign and width
@@ -98,11 +96,10 @@  test_int()
   VERIFY( is_std_format_spec_for<int>("0") );
   VERIFY( ! is_std_format_spec_for<int>("00d") );
   VERIFY( is_std_format_spec_for<int>("01d") );
-  VERIFY( ! is_std_format_spec_for<int>("0{}d") );
   VERIFY( ! is_std_format_spec_for<int>("0{1}d") );
-  VERIFY(( is_std_format_spec_for<int, 2>("0{}d") ));
-  VERIFY(( ! is_std_format_spec_for<int, 2>("0{1}d") ));
-  VERIFY(( is_std_format_spec_for<int, 2, false>("0{1}d") ));
+  VERIFY(( is_std_format_spec_for<int>("0{}d") ));
+  VERIFY(( ! is_std_format_spec_for<int>("0{1}d") ));
+  VERIFY(( is_std_format_spec_for<int, false>("0{1}d") ));
   VERIFY( is_std_format_spec_for<int>("1") );
   VERIFY( is_std_format_spec_for<int>("-1") ); // sign and width
   VERIFY( ! is_std_format_spec_for<int>(".") );
@@ -193,24 +190,20 @@  test_float()
   VERIFY( is_std_format_spec_for<float>("0") );
   VERIFY( ! is_std_format_spec_for<float>("00f") );
   VERIFY( is_std_format_spec_for<float>("01f") );
-  VERIFY( ! is_std_format_spec_for<float>("0{}f") );
+  VERIFY( is_std_format_spec_for<float>("0{}f") );
   VERIFY( ! is_std_format_spec_for<float>("0{1}f") );
-  VERIFY(( is_std_format_spec_for<float, 2>("0{}f") ));
-  VERIFY(( ! is_std_format_spec_for<float, 2>("0{1}f") ));
-  VERIFY(( is_std_format_spec_for<float, 2, false>("0{1}f") ));
+  VERIFY( ! is_std_format_spec_for<float>("0{1}f") );
+  VERIFY(( is_std_format_spec_for<float, false>("0{1}f") ));
   VERIFY( is_std_format_spec_for<float>("1") );
   VERIFY( is_std_format_spec_for<float>("-1") ); // sign and width
   VERIFY( ! is_std_format_spec_for<float>(".") );
   VERIFY( is_std_format_spec_for<float>(".1") );
-  VERIFY( ! is_std_format_spec_for<float>(".{}") );
+  VERIFY( is_std_format_spec_for<float>(".{}") );
   VERIFY( ! is_std_format_spec_for<float>(".{1}") );
-  VERIFY(( is_std_format_spec_for<float, 2>(".{}") ));
-  VERIFY(( ! is_std_format_spec_for<float, 2>(".{1}") ));
-  VERIFY(( is_std_format_spec_for<float, 2, false>(".{1}") ));
-  VERIFY(( ! is_std_format_spec_for<float, 2>("{}.{}") ));
-  VERIFY(( is_std_format_spec_for<float, 3>("{}.{}") ));
-  VERIFY(( is_std_format_spec_for<float, 2, false>("{1}.{1}") ));
-  VERIFY(( is_std_format_spec_for<float, 3, false>("{2}.{1}") ));
+  VERIFY(( is_std_format_spec_for<float, false>(".{1}") ));
+  VERIFY( is_std_format_spec_for<float>("{}.{}") );
+  VERIFY(( is_std_format_spec_for<float, false>("{1}.{1}") ));
+  VERIFY(( is_std_format_spec_for<float, false>("{2}.{1}") ));
   VERIFY( ! is_std_format_spec_for<float>("c") );
   VERIFY( ! is_std_format_spec_for<float>("b") );
   VERIFY( ! is_std_format_spec_for<float>("B") );
@@ -302,11 +295,9 @@  test_string()
   VERIFY( ! is_std_format_spec_for<const char*>("-1s") );
   VERIFY( ! is_std_format_spec_for<const char*>(".") );
   VERIFY( is_std_format_spec_for<const char*>(".1") );
-  VERIFY( ! is_std_format_spec_for<const char*>(".{}") );
-  VERIFY(( ! is_std_format_spec_for<const char*, 1, false>(".{1}") ));
-  VERIFY(( is_std_format_spec_for<const char*, 1, false>(".{0}") ));
-  VERIFY(( is_std_format_spec_for<const char*, 2>(".{}") ));
-  VERIFY(( is_std_format_spec_for<const char*, 2, false>(".{1}") ));
+  VERIFY( is_std_format_spec_for<const char*>(".{}") );
+  VERIFY(( is_std_format_spec_for<const char*, false>(".{0}") ));
+  VERIFY(( is_std_format_spec_for<const char*, false>(".{1}") ));
   VERIFY( ! is_std_format_spec_for<const char*>("c") );
   VERIFY( ! is_std_format_spec_for<const char*>("b") );
   VERIFY( ! is_std_format_spec_for<const char*>("B") );