Formatting thread::id, stacktrace_entry, and basic_stacktrace

[JMazurkiewicz]

• Add _Fill_align_and_width_specs, _Fill_align_and_width_specs_setter and _Fill_align_and_width_formatter types. They are use by formatters that accept only fill-and-align and width fields (thread::id and stacktrace_entry in this PR).
• Move std::call_once from <mutex> to <xcall_once.h> to avoid circular dependency. I hope this is OK.
• Implement WG21-P2693R1: formatters for thread::id, stacktrace_entry and basic_stacktrace.
• Fix operator<<(thread::id) in all supported C++ modes. See LLVM-62073 for bug report and @mordante's rationale.
• Add tests.
• Close P2693R1 Formatting thread::id And stacktrace #3447.

[cpplearner]

Could _Specs_setter, _Chrono_specs_setter and the new _Fill_align_and_width_formatter share some common members? It's a bit awkward to have three identical definitions of _On_fill and others.

(Also, P2286R8 adds range-format-spec and tuple-format-spec, which are basically fill-and-align + width + type-specific options. It would be nice if the existing machinery can be reused when dealing with their fill, align, and width.)

[JMazurkiewicz]

Could _Specs_setter, _Chrono_specs_setter and the new _Fill_align_and_width_formatter share some common members? It's a bit awkward to have three identical definitions of _On_fill and others.

I agree, unifying those setters is necessary. I'd leave it for another PR though, since it would require editing another header (<chrono>) and this PR is already big enough.

[barcharcraz]

I suspect this could be unified with _Basic_format_specs and _Dynamic_format_specs, I need to work out what that would actually look like though, and it probably requires nontrivial inheritance.

[barcharcraz]

At first glance things look pretty good, I'll see if I can figure out how you might be able to eliminate all the duplicate specs setter code without changing a bunch of chrono stuff next week.

[StephanTLavavej]

Thanks! I'll validate and push changes...

[barcharcraz]

In order to prevent the code duplication here we'd have to rearrange _Basic_format_specs and _Dynamic_format_specs, and I'm not sure we can do that without ABI impact.

If we want to unify things we could do something like:

template<class _CharT>
struct _Really_basic_format_specs {
    int _Width = -1;
    _Fmt_align _Alignment = _Fmt_align::_None;
    uint8_t _Fill_length  = 1;
    // At most one codepoint (so one char32_t or four utf-8 char8_t).
    _CharT _Fill[4 / sizeof(_CharT)] = {' '};
};



template <class _CharT>
struct _Basic_format_specs : _Really_basic_format_specs<_CharT> {
    int _Precision        = -1;
    char _Type            = '\0';
    _Fmt_sign _Sgn        = _Fmt_sign::_None;
    bool _Alt             = false;
    bool _Localized       = false;
    bool _Leading_zero    = false;
};

struct _Dynamic_width_format_specs {
    int _Dynamic_width_index = -1;
};

// used by thread::id and stacktrace_entry formatters
template <class _CharT>
struct _Fill_align_and_width_specs : _Really_basic_format_specs<_CharT>, _Dynamic_width_format_specs { };

template <class _CharT>
struct _Dynamic_format_specs : _Basic_format_specs<_CharT>, _Dynamic_width_format_specs {
    int _Dynamic_precision_index = -1;
};

The specs setter classes could be slightly refactored to use either CRTP or to just have a collection of static methods taking the specs to fill in as the first parameter (basically making the setter even closer to members of the specs classes).

Again, this changes the order of the specs members, and the spec class is included by-value as a member of all the formatters, which users are allowed to derive from. This is also a bigger refactor, so I'm comfortable leaving it for later.

[StephanTLavavej]

Again, this changes the order of the specs members, and the spec class is included by-value as a member of all the formatters, which users are allowed to derive from.

That sounds like an unavoidably ABI-breaking change. Sounds like everyone's happy with a follow-up investigation; I suspect that no change will happen as a result.

[StephanTLavavej]

The failure appears to be consistently happening in fmt("{:{}}", frame, width) but I haven't been able to figure out what the problem is.

[StephanTLavavej]

I've pushed a defensive commit to use clamp() to avoid fill_width ever being negative. Let's see if this helps...

[StephanTLavavej]

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[StephanTLavavej]

Thanks for implementing this feature! ⚙️ 😻 ✅