[libc++][format] Improve an exception message.

[Mordante]

Summary

The fix in D116381 makes an existing exception message wrong. This
improves the message and fixes the associated unit tests.

Note other message can be also be improved, but that will be done later.
Changing these messages may cause merge conflicts with other patches
that are under review or WIP.

Depends on D116381

[• Quuxplusone]

Seems fine to me!

[ldionne]

I'm a bit confused -- I thought the error messages was supposed to mention that the arg-id shouldn't have leading zeros?

[Mordante]

This as a generic message error message and a leading zero is one of the possible errors. As you can see in the tests it also rejects {9a, {9:. I don't think it's useful to add an extra code path to add more details for the exact error. There's also no specific error for {1 at the end of input. Do you think it's useful to show the expected format? Something along the lines of "Arg-id doesn't match {(0|[1-9][0-9]*)}".

[ldionne]

LGTM but I think it would be worth looking into whether we can improve all error messages, independently. IIUC this is at least an improvement in that it makes the error messages "not lie".

[Mordante]

Rebased to poke CI.

libcxx/include/__format/parser_std_format_spec.h

__parse_arg_id(const _CharT* __begin, const _CharT* __end, auto& __parse_ctx) {
   // validation for the pre-conditions and post-conditions.
   if (__begin == __end)
     __throw_format_error("End of input while parsing format-spec arg-id");
 
   __format::__parse_number_result __r =
       __format::__parse_arg_id(__begin, __end, __parse_ctx);
 
   if (__r.__ptr == __end || *__r.__ptr != _CharT('}'))
-    __throw_format_error("A format-spec arg-id should terminate at a '}'");
+    __throw_format_error("Invalid arg-id");
 
   ++__r.__ptr;
   return __r;
 }
 
 template <class _Context>
 _LIBCPP_HIDE_FROM_ABI constexpr uint32_t
 __substitute_arg_id(basic_format_arg<_Context> __arg) {

libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_bool.pass.cpp

test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("2147483648"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("5000000000"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("10000000000"));
 
   test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id",
                                 CSTR("{"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{0"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{0"));
   test_exception<Parser<CharT>>(
       "The arg-id of the format-spec starts with an invalid character",
       CSTR("{a"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{1"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9:"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9a"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{1"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9:"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9a"));
 
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   // Note the static_assert tests whether the arg-id is valid.
   // Therefore the following should be true arg-id < __format::__number_max.
   test({.width = 2'147'483'646, .width_as_arg = true}, 12,
        CSTR("{2147483646}}"));
   test_exception<Parser<CharT>>(

libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_char.pass.cpp

test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("2147483648"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("5000000000"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("10000000000"));
 
   test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id",
                                 CSTR("{"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{0"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{0"));
   test_exception<Parser<CharT>>(
       "The arg-id of the format-spec starts with an invalid character",
       CSTR("{a"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{1"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9:"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9a"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{1"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9:"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9a"));
 
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   // Note the static_assert tests whether the arg-id is valid.
   // Therefore the following should be true arg-id < __format::__number_max.
   test({.width = 2'147'483'646, .width_as_arg = true}, 12,
        CSTR("{2147483646}}"));
   test_exception<Parser<CharT>>(

libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_integer.pass.cpp

test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("2147483648"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("5000000000"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("10000000000"));
 
   test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id",
                                 CSTR("{"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{0"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{0"));
   test_exception<Parser<CharT>>(
       "The arg-id of the format-spec starts with an invalid character",
       CSTR("{a"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{1"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9:"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9a"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{1"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9:"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9a"));
 
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   // Note the static_assert tests whether the arg-id is valid.
   // Therefore the following should be true arg-id < __format::__number_max.
   test({.width = 2'147'483'646, .width_as_arg = true}, 12,
        CSTR("{2147483646}}"));
   test_exception<Parser<CharT>>(

libcxx/test/libcxx/utilities/format/format.string/format.string.std/std_format_spec_string.pass.cpp

test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("2147483648"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("5000000000"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR("10000000000"));
 
   test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id",
                                 CSTR("{"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{0"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{0"));
   test_exception<Parser<CharT>>(
       "The arg-id of the format-spec starts with an invalid character",
       CSTR("{a"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{1"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9:"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR("{9a"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{1"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9:"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR("{9a"));
 
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   // Note the static_assert tests whether the arg-id is valid.
   // Therefore the following should be true arg-id < __format::__number_max.
   test({.width = 2'147'483'646, .width_as_arg = true}, 12,
        CSTR("{2147483646}}"));
   test_exception<Parser<CharT>>(
constexpr void test() {
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large", CSTR(".5000000000"));
   test_exception<Parser<CharT>>(
       "The numeric value of the format-spec is too large",
       CSTR(".10000000000"));
 
   test_exception<Parser<CharT>>("End of input while parsing format-spec arg-id",
                                 CSTR(".{"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR(".{0"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR(".{0"));
   test_exception<Parser<CharT>>(
       "The arg-id of the format-spec starts with an invalid character",
       CSTR(".{a"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR(".{1"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR(".{9"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR(".{9:"));
-  test_exception<Parser<CharT>>(
-      "A format-spec arg-id should terminate at a '}'", CSTR(".{9a"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR(".{1"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR(".{9"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR(".{9:"));
+  test_exception<Parser<CharT>>("Invalid arg-id", CSTR(".{9a"));
 
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   // Note the static_assert tests whether the arg-id is valid.
   // Therefore the following should be true arg-id < __format::__number_max.
   test({.precision = 2'147'483'646, .precision_as_arg = true}, 13,
        CSTR(".{2147483646}}"));
   test_exception<Parser<CharT>>(

libcxx/test/std/utilities/format/format.functions/format_tests.h

check_exception(
       STR("hello {:{}}"), world, universe);
   check_exception(
       "Using manual argument numbering in automatic argument numbering mode",
       STR("hello {:{0}}"), world, 1);
   check_exception(
       "Using automatic argument numbering in manual argument numbering mode",
       STR("hello {0:{}}"), world, 1);
   // Arg-id may not have leading zeros.
-  check_exception(
-      "A format-spec arg-id should terminate at a '}'",
-      STR("hello {0:{01}}"), world, 1);
+  check_exception("Invalid arg-id", STR("hello {0:{01}}"), world, 1);
 
   // *** precision ***
 #if _LIBCPP_VERSION
   // This limit isn't specified in the Standard.
   static_assert(std::__format::__number_max == 2'147'483'647,
                 "Update the assert and the test.");
   check_exception("The numeric value of the format-spec is too large",
                   STR("{:.2147483648}"), world);
check_exception(
       STR("hello {:.{}}"), world, universe);
   check_exception(
       "Using manual argument numbering in automatic argument numbering mode",
       STR("hello {:.{0}}"), world, 1);
   check_exception(
       "Using automatic argument numbering in manual argument numbering mode",
       STR("hello {0:.{}}"), world, 1);
   // Arg-id may not have leading zeros.
-  check_exception(
-      "A format-spec arg-id should terminate at a '}'",
-      STR("hello {0:.{01}}"), world, 1);
+  check_exception("Invalid arg-id", STR("hello {0:.{01}}"), world, 1);
 
   // *** locale-specific form ***
   check_exception("The format-spec should consume the input or end with a '}'",
                   STR("hello {:L}"), world);
 
   // *** type ***
   for (const auto& fmt : invalid_types<CharT>("s"))
     check_exception(