[libcxx-commits] [libcxx] [libc++][format] Don't treat a closing '}' as part of format-spec (PR #81305)

[Po-yao Chang via libcxx-commits]

================
@@ -251,7 +251,7 @@ __handle_replacement_field(_Iterator __begin, _Iterator __end, _ParseCtx& __pars
   if (__r.__last == __end)
     std::__throw_format_error("The argument index should end with a ':' or a '}'");
 
-  bool __parse = *__r.__last == _CharT(':');
+  bool __parse = __parse_ctx.__should_parse() = *__r.__last == _CharT(':');
----------------
poyaoc97 wrote:

After reading your comment and the Standard, I'm thinking maybe doing an early return just before calling `__parse_fill_align` is the right thing to do.
https://github.com/llvm/llvm-project/blob/b45de48be24695b613f48ed21bb35f844454193b/libcxx/include/__format/parser_std_format_spec.h#L355-L361
```diff
-     if (__begin == __end)
+     if (__begin == __end || *__begin == _CharT('}'))
```

According to [[format.string.general]/1](https://eel.is/c++draft/format.string.general#1), `}` is definitely not the start of `format-spec` so when we see a `}` at the start of `__parse_fill_align`, that must belong to `replacement-field` and the `format-spec` is empty. Specialized formatters (`__validate_fill_character` that is) shouldn't even have a say about that `}`, [[format.string.general]/5](https://eel.is/c++draft/format.string.general#5).

For example, we can parse `{:}<}` as having an empty `format-spec` followed by an invalid escape sequence rather than having an invalid `fill` followed by an `align` and a closing brace. The ill-fomed-ness doesn't change but the exception.what() does, so we need to fix the tests checking this if we are going this route.

All library types' format specifications seem to be optional and maybe that's designed like so to allow this way of parsing.

https://github.com/llvm/llvm-project/pull/81305