On Tue, 14 Mar 2023 at 10:51, Daniel Krügler wrote:

> Apologies for the late response:
>
>
I only just committed the change, so it's not delayed :-)



> What about changing the test to check for __cpp_inline_variables or
> combining it with __cpp_variable_templates instead?
>
>
We could do that, but it would complicate their use.

Currently they're only used in C++17 code (chrono::floor etc.) and C++20
code (chrono::hh_mm_ss etc. and chrono formatters). We know it's OK for
C++17 and C++20 code to use __is_duration_v and __is_time_point_v because
they're defined for C++17 and later.

If we change them to be defined for __cpp_inline_variables &&
__cpp_variable_templates then what changes? It should be safe to assume we
can still use them in C++17 and C++20 code, but could we also use them
elsewhere, e.g. in C++14 code such as chrono::literals? Maybe, but only if
__cpp_inline_variables is defined for C++14 mode, and if it's not, then
we'd need something like:

#if __cplusplus >= 201402L
  template<typename _Dur>
#if __cpp_inline_variables
    enable_if_t<__is_duration_v<_Dur>, _Dur>
#else
    enable_if_t<__is_duration<_Dur>::value, _Dur>
#endif
    foo(const _Dur&);
#endif

And this is not an improvement over simply:

#if __cplusplus >= 201402L
  template<typename _Dur>
    enable_if_t<__is_duration<_Dur>::value, _Dur>
    foo(const _Dur&);
#endif

So I don't see why we would want to do it. I think it was a mistake for me
to ever make them depend on __cpp_variable_templates, instead of just
depending on C++17. I think it's better to fix that mistake.

Reply via email to