[committed] libstdc++: Fix preprocessor condition for inline variables

[committed] libstdc++: Fix preprocessor condition for inline variables

[Jonathan Wakely]

Tested x86_64-linux. Pushed to trunk.

-- >8 --

Although variable templates are valid in C++14, inline ones aren't.
These are only used in C++17 (or later) code, so they don't need to be
defined for C++14.

libstdc++-v3/ChangeLog:

	* include/bits/chrono.h (__is_duration_v, __is_time_point_v):
	Only define for C++17 and later.
---
 libstdc++-v3/include/bits/chrono.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/include/bits/chrono.h b/libstdc++-v3/include/bits/chrono.h
index b2e4f4c33a8..fb99fe5eed7 100644
--- a/libstdc++-v3/include/bits/chrono.h
+++ b/libstdc++-v3/include/bits/chrono.h
@@ -244,7 +244,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       using __disable_if_is_duration
 	= typename enable_if<!__is_duration<_Tp>::value, _Tp>::type;
 
-#if __cpp_variable_templates
+#if __cplusplus >= 201703L
     template<typename _Tp>
       inline constexpr bool __is_duration_v = false;
     template<typename _Rep, typename _Period>
-- 
2.39.2

Re: [committed] libstdc++: Fix preprocessor condition for inline variables

[Daniel Krügler]

Am Di., 14. März 2023 um 11:32 Uhr schrieb Jonathan Wakely via
Libstdc++ <libstdc++@gcc.gnu.org>:
>
> Tested x86_64-linux. Pushed to trunk.
>
> -- >8 --
>
> Although variable templates are valid in C++14, inline ones aren't.
> These are only used in C++17 (or later) code, so they don't need to be
> defined for C++14.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/chrono.h (__is_duration_v, __is_time_point_v):
>         Only define for C++17 and later.
> ---
>  libstdc++-v3/include/bits/chrono.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/include/bits/chrono.h b/libstdc++-v3/include/bits/chrono.h
> index b2e4f4c33a8..fb99fe5eed7 100644
> --- a/libstdc++-v3/include/bits/chrono.h
> +++ b/libstdc++-v3/include/bits/chrono.h
> @@ -244,7 +244,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        using __disable_if_is_duration
>         = typename enable_if<!__is_duration<_Tp>::value, _Tp>::type;
>
> -#if __cpp_variable_templates
> +#if __cplusplus >= 201703L
>      template<typename _Tp>
>        inline constexpr bool __is_duration_v = false;
>      template<typename _Rep, typename _Period>
> --
> 2.39.2

Apologies for the late response:

What about changing the test to check for __cpp_inline_variables or
combining it with __cpp_variable_templates instead?

Thanks,

- Daniel

Re: [committed] libstdc++: Fix preprocessor condition for inline variables

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

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.

Re: [committed] libstdc++: Fix preprocessor condition for inline variables

[Daniel Krügler]

Am Di., 14. März 2023 um 12:02 Uhr schrieb Jonathan Wakely <jwakely@redhat.com>:
>
> 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.

Sounds reasonable, thanks.

- Daniel