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

[Mark de Wever 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(':');
----------------
mordante wrote:

I did some more investigation, this seems to be the wrong approach. It solves
`std::println("{}>42", std::thread::id{});`, but fails with `std::println("{:}>42", std::thread::id{});`.

I did a bit more testing on [godbolt](https://godbolt.org/z/shfbaroj5) and other cases for libc++ fail too.

We should take a different approach to fix this
https://github.com/llvm/llvm-project/blob/main/libcxx/include/__format/parser_std_format_spec.h#L580
```
    if (__use_range_fill && (__fill == _CharT('{') || __fill == _CharT('}') || __fill == _CharT(':')))
      std::__throw_format_error("The fill option contains an invalid value");
    else if (__fill == _CharT('{') || __fill == _CharT('}'))
      std::__throw_format_error("The fill option contains an invalid value");
```
here we should not test for '}' instead 

https://github.com/llvm/llvm-project/blob/main/libcxx/include/__format/parser_std_format_spec.h#L609
```
 __validate_fill_character(*__begin, __use_range_fill);
```
should first test whether the fill character is `_CharT('}')` and return `false` when this condition is true.

I would like to see tests for the cases this fixes.

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