adding a default format for std::chrono::time_point<std::chrono::syst…

[sunmy2019]

For #2319, I add a defalut spec when the format is {}.

Here I use function overload to avoid if constexpr or different template specializations. If you want to support more char types, you can provide with more overload resolutions, or use some template metaprogramming techniques (to restrict sizeof(Char) ).

[sunmy2019]

I was presuming basic_string_view has a constructor that takes in const char (&)[N].

I look up the source today. It turns out there isn't one.

[sunmy2019]

I accidently merged the upstream commits. Now I reset to 12aacc2

[sunmy2019]

do you mean default?

fixed, thanks

[vitaut]

Thanks for the PR. Minor comments inline, otherwise LGTM.

[foonathan]

Here I use function overload to avoid if constexpr or different template specializations. If you want to support more char types, you can provide with more overload resolutions, or use some template metaprogramming techniques (to restrict sizeof(Char) ).

Note that in principle, one can use something like constexpr const CharT format[] = {'%', 'Y', '-', ... } and automatically supports every character type. I'm not sure if it's worth it, however.

[vitaut]

Here I use function overload to avoid if constexpr or different template specializations. If you want to support more char types, you can provide with more overload resolutions, or use some template metaprogramming techniques (to restrict sizeof(Char) ).

Note that in principle, one can use something like constexpr const CharT format[] = {'%', 'Y', '-', ... } and automatically supports every character type. I'm not sure if it's worth it, however.

I don't think it'll work in this particular case because we need a static string and static constexpr is not supported in constexpr functions (sigh).

[foonathan]

It could be made a static constexpr at class scope.

[vitaut]

It could be made a static constexpr at class scope.

Good idea, let's do that instead of my earlier suggestion.

[sunmy2019]

But using static constexpr in aab4a1b would cause failure on C++11/14, and succeed on C++17/20.
The linker complained about

undefined reference to `fmt::v7::formatter<std::chrono::time_point<std::chrono::_V2::system_clock,
std::chrono::duration<long, std::ratio<1l, 1000000000l> > >, wchar_t, void>::default_spec'

[sunmy2019]

But using static constexpr in aab4a1b would cause failure on C++11/14, and succeed on C++17/20.
The linker complained about

undefined reference to `fmt::v7::formatter<std::chrono::time_point<std::chrono::_V2::system_clock,
std::chrono::duration<long, std::ratio<1l, 1000000000l> > >, wchar_t, void>::default_spec'

Inline variables are introduced in C++17, before C++17 we seem to have to generate the symbol somewhere.

GCC and Clang add supports for inline variables under C++17 as an extension. It seems a failure on MSVC and on older versions of GCC/Clang.

[alexezeder]

I need to use static constexpr inline, since it's implicit inline since C++17.

static inline class variable is a C++17 feature. constexpr implies inline since C++11.
There is an example in {fmt} how to use static constexpr class variables and conform to C++11/14/17+.

fmt/include/fmt/format.h

Line 842 in 36c2948

FMT_API static constexpr const char digits[][2] = {

fmt/include/fmt/format-inl.h

Lines 148 to 149 in 36c2948

	#if __cplusplus < 201703L
	template <typename T> constexpr const char basic_data<T>::digits[][2];

[sunmy2019]

I need to use static constexpr inline, since it's implicit inline since C++17.

static inline class variable is a C++17 feature. constexpr implies inline since C++11.
There is an example in {fmt} how to use static constexpr class variables and conform to C++11/14/17+.

fmt/include/fmt/format.h

Line 842 in 36c2948

FMT_API static constexpr const char digits[][2] = {

fmt/include/fmt/format-inl.h

Lines 148 to 149 in 36c2948

	#if __cplusplus < 201703L
	template <typename T> constexpr const char basic_data<T>::digits[][2];

Does this mean I need to create a chrono-inl.h to provide with both header-only library and (non header-only) library? I also need to add a translation unit like src/format.cc, and also a change to the building script.

This seems to be a huge change beyond my scope.

[alexezeder]

This seems to be a huge change beyond my scope.

Can't we use here something like:

template<typename A, typename B>
class formatter<A, B> {
  static constexpr auto var = { /* initializer here */ };
};
template <typename A, typename B> constexpr B formatter<A, B>::var[]; 

For me, it's a big question why this code:

fmt/include/fmt/core.h

Lines 1686 to 1691 in 36c2948

	static constexpr unsigned long long desc =
	(is_packed ? detail::encode_types<Context, Args...>()
	: detail::is_unpacked_bit | num_args) |
	(num_named_args != 0
	? static_cast<unsigned long long>(detail::has_named_args_bit)
	: 0);

does not require any out-of-class definitions.

[sunmy2019]

Can't we use here something like:

template<typename A, typename B>
class formatter<A, B> {
  static constexpr auto var = { /* initializer here */ };
};
template <typename A, typename B> constexpr B formatter<A, B>::var[]; 

Let's have a try.

[foonathan]

does not require any out-of-class definitions.

You only need a definition if something is "ODR used". Reading a constant integer is not an ODR use, but some part of the code that deals with string literals seems to ODR use the format spec.

Can't we use here something like:

This should definitely work.

[sunmy2019]

Oops, I find my implementation cannot handle compile time string correctly for C++17/20.

The format will be parsed empty in

fmt::print(FMT_COMPILE("{}"), std::chrono::system_clock::now());

But it works correctly in

fmt::print(FMT_COMPILE("{:}"), std::chrono::system_clock::now());

I don't know what's behind the scene, but I guess the parse function was never entered when using {}.

So I move the assignment this->specs to the default constructor in 0f8dc8c .

I tested it on my g++-11 with C++11/14/17/20 standard.

[vitaut]

Thank you!