microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.14k stars 1.5k forks source link

`<format>`: Is `c` really a cromulent integer presentation type? #4918

Open StephanTLavavej opened 2 months ago

StephanTLavavej commented 2 months ago

https://godbolt.org/z/vac7s4rKn

#include <cassert>
#include <format>
#include <string>
using namespace std;

int main() {
    // Everyone accepts:
    assert(format("{0:#}", 77) == "77");
    assert(format("{0:#d}", 77) == "77");
    assert(format("{0:#b}", 77) == "0b1001101");
    assert(format("{0:#B}", 77) == "0B1001101");
    assert(format("{0:#o}", 77) == "0115");
    assert(format("{0:#x}", 77) == "0x4d");
    assert(format("{0:#X}", 77) == "0X4D");
    assert(format("{0:c}", 77) == "M");

    // libstdc++ 14.2 accepts, libc++ 18.1 and microsoft/STL 17.10 reject:
    assert(format("{0:#c}", 77) == "M");

    // 77 is an int, which is an arithmetic type other than charT and bool.
    // 'c' is an integer presentation type.
    // Therefore, I conclude that formatting 77 with "{0:#c}" is doubly valid.
    // Here's the Standardese:

    // N4988 [format.string.std]/7:
    // "The # option causes the alternate form to be used for the conversion.
    // This option is valid for arithmetic types other than charT and bool
    // or when an integer presentation type is specified, and not otherwise."

    // N4988 [format.string.std]/21:
    // "The available integer presentation types for integral types other than bool and charT
    // are specified in Table 72."

    // Table 72 - Meaning of type options for integer types [tab:format.type.int]
    // Type | Meaning
    // -----|--------
    // b    | to_chars(first, last, value, 2); the base prefix is 0b.
    // B    | The same as b, except that the base prefix is 0B.
    // c    | Copies the character static_cast<charT>(value) to the output.
    //      | Throws format_error if value is not in the range of representable values for charT.
    // [...]
}

With VS 2022 17.12 Preview 1:

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od /diagnostics:caret meow.cpp
meow.cpp
meow.cpp(18,5): error C7595: 'std::basic_format_string<char,int>::basic_format_string': call to immediate function is not a constant expression
    assert(format("{0:#c}", 77) == "M");
    ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1977,32): note: failure was caused by call of undefined function or one not declared 'constexpr'
            _Throw_format_error("Hash/sign modifier requires an arithmetic presentation type");
                               ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1977,32): note: see usage of 'std::_Throw_format_error'
meow.cpp(18,5): note: the call stack of the evaluation (the oldest call first) is
    assert(format("{0:#c}", 77) == "M");
    ^
meow.cpp(18,5): note: while evaluating function 'std::basic_format_string<char,int>::basic_format_string<char[7]>(const _Ty (&))'
        with
        [
            _Ty=char [7]
        ]
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(3696,33): note: while evaluating function 'void std::_Parse_format_string<char,std::__p2286::_Format_checker<char,int>>(std::basic_string_view<char,std::char_traits<char>>,_HandlerT &&)'
        with
        [
            _HandlerT=std::__p2286::_Format_checker<char,int>
        ]
            _Parse_format_string(_Str, _Format_checker<_CharT, remove_cvref_t<_Args>...>{_Str, _Arg_types});
                                ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1551,42): note: while evaluating function 'const _CharT *std::_Parse_replacement_field<char,_HandlerT&>(const _CharT *,const _CharT *,std::__p2286::_Format_checker<_CharT,int>&)'
        with
        [
            _CharT=char,
            _HandlerT=std::__p2286::_Format_checker<char,int>
        ]
        _First = _Parse_replacement_field(_OpeningCurl, _Last, _Handler);
                                         ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1500,47): note: while evaluating function 'const _CharT *std::__p2286::_Format_checker<_CharT,int>::_On_format_specs(const size_t,const _CharT *,const _CharT *)'
        with
        [
            _CharT=char
        ]
            _First = _Handler._On_format_specs(_Adapter._Arg_id, _First + 1, _Last);
                                              ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(3592,42): note: while evaluating function 'std::_String_view_iterator<_Traits> std::__p2286::_Compile_time_parse_format_specs<int,std::basic_format_parse_context<char>>(_ParseContext &)'
        with
        [
            _Traits=std::char_traits<char>,
            _ParseContext=std::basic_format_parse_context<char>
        ]
            auto _Iter = _Parse_funcs[_Id](_Parse_context); // TRANSITION, VSO-1451773 (workaround: named variable)
                                         ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(3568,23): note: while evaluating function 'std::_String_view_iterator<_Traits> std::__p2286::_Formatter_base<int,_CharT,std::_Basic_format_arg_type::_Int_type>::parse<std::basic_format_parse_context<_CharT>>(_Pc &)'
        with
        [
            _Traits=std::char_traits<char>,
            _CharT=std::__p2286::_Compile_time_parse_format_specs::_CharT,
            _Pc=std::basic_format_parse_context<char>
        ]
    return _Formatter.parse(_Pc);
                      ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\__msvc_formatter.hpp(160,16): note: while evaluating function 'std::_String_view_iterator<_Traits> std::__p2286::_Formatter_base_parse<std::_Basic_format_arg_type::_Int_type,_CharT,_Pc>(std::_Dynamic_format_specs<_CharT> &,_Pc &)'
        with
        [
            _Traits=std::char_traits<char>,
            _CharT=std::__p2286::_Compile_time_parse_format_specs::_CharT,
            _Pc=std::basic_format_parse_context<char>
        ]
        return _Formatter_base_parse<_ArgType>(_Specs, _ParseCtx);
               ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(3656,41): note: while evaluating function 'const _CharT *std::_Parse_format_specs<_CharT,std::_Specs_checker<std::_Dynamic_specs_handler<_Pc>>&>(const _CharT *,const _CharT *,_Callbacks_type)'
        with
        [
            _CharT=char,
            _Pc=std::basic_format_parse_context<char>,
            _Callbacks_type=std::_Specs_checker<std::_Dynamic_specs_handler<std::basic_format_parse_context<char>>> &
        ]
    const auto _It = _Parse_format_specs(_ParseCtx._Unchecked_begin(), _ParseCtx._Unchecked_end(), _Handler);
                                        ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1463,28): note: while evaluating function 'void std::_Specs_checker<std::_Dynamic_specs_handler<_Pc>>::_On_type<_CharT>(_CharT)'
        with
        [
            _Pc=std::basic_format_parse_context<char>,
            _CharT=char
        ]
        _Callbacks._On_type(*_First);
                           ^
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\format(1977,32): note: while evaluating function 'void std::_Throw_format_error(const char *const )'
            _Throw_format_error("Hash/sign modifier requires an arithmetic presentation type");
                               ^

https://github.com/microsoft/STL/blob/8af2cc499c94271c8d12baeb45e840f7160e082d/stl/inc/format#L1859-L1861

I believe that all of hash, sign, and zero are affected:

https://github.com/microsoft/STL/blob/8af2cc499c94271c8d12baeb45e840f7160e082d/stl/inc/format#L1971-L1985

Someone should double-check my interpretation of the Standardese here, I'm not an expert.

Related: https://github.com/llvm/llvm-project/issues/106136

CaseyCarter commented 2 months ago

I completely agree with your interpretation of the Standard, and believe we should label this "LWG issue needed" instead of "bug". I think it's nonsensical to allow :#c with the same meaning as :c, and probably not the intended design.

@vitaut any opinion? I see fmt allows :#c (https://godbolt.org/z/GxGKhPh97).

I may be biased by my belief that most format strings are statically determined, which suggests to me we want to tell the user they're being silly rather than silently doing nothing when they add a # here. (A mechanism to emit arbitrary library warnings during evaluation of constant expressions would be nice. Also a pony.)

StephanTLavavej commented 2 months ago

I observe a few things:

I think I'm definitely in favor of "LWG issue needed" and I'd be fine with "libc++ and MSVC's STL were doing the right thing all along".

vitaut commented 2 months ago

# is supposed to be used with numeric presentation types and doesn't make much sense with c. We could ban it even though it's pretty harmless.