diff mbox series

[C++] Implementing P0767 - deprecate POD

Message ID CANHA4OgUjYbXmqN=M0Rvc+=ju2obD0-JH+s8Xbj6AeisCL7Ztw@mail.gmail.com
State New
Headers show
Series [C++] Implementing P0767 - deprecate POD | expand

Commit Message

JeanHeyd Meneide Dec. 4, 2019, 1:30 a.m. UTC 
This patch implements deprecate POD for the C++ Standard Library,
bringing libstdc++ that much closer to 2020 conformance 🎉!

Hilariously, a small bug in the [[deprecated]] warning message was
found while implementing this patch, which drove me a bit insane for a
good 10 minutes until I realized what was going on:
https://godbolt.org/z/WwBNUx

Either way, here you go! Someone will probably have to fix the
template-class [[deprecated("foo")]] bug at some point too.

2019-12-03  JeanHeyd "ThePhD" Meneide  <phdofthehouse@gmail.com>

        * include/bits/c++config: Add new _GLIBCXX20_DEPRECATED macro.
        * include/std/type_traits: Deprecate is_pod with message.
        * testuite/20_util/is_pod/deprecated-2a.cc (new): test deprecation

Comments

Jonathan Wakely Dec. 4, 2019, 9:47 p.m. UTC | #1 
On 03/12/19 20:30 -0500, JeanHeyd Meneide wrote:
>This patch implements deprecate POD for the C++ Standard Library,

Thanks.

>bringing libstdc++ that much closer to 2020 conformance 🎉!

N.B. adding the attribute is not required for conformance.

>+#if defined(__DEPRECATED) && (__cplusplus > 201703L)
>+# define _GLIBCXX20_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]

Does this need to us __VA_ARGS__? Won't it always be a single
preprocessor token, specifically, a string literal?

>+#else
>+# define _GLIBCXX20_DEPRECATED(...)
>+#endif
>+
> // Macros for ABI tag attributes.
> #ifndef _GLIBCXX_ABI_TAG_CXX11
> # define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
>diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
>index 8e787a994c3..91269d1bd02 100644
>--- a/libstdc++-v3/include/std/type_traits
>+++ b/libstdc++-v3/include/std/type_traits
>@@ -685,10 +685,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	"template argument must be a complete class or an unbounded array");
>     };
>
>-  /// is_pod
>+  /// is_pod (deprecated C++2a)
>   // Could use is_standard_layout && is_trivial instead of the builtin.
>   template<typename _Tp>
>-    struct is_pod
>+    struct
>+    _GLIBCXX20_DEPRECATED("is_pod<Type> is deprecated in C++20: use is_standard_layout<Type> && is_trivial<Type> instead")

That isn't a valid expression, it should probably say:

"use is_standard_layout_v<Type> && is_trivial_v<Type> instead"

i.e. recommend the variable templates.

I think I'd prefer "T" rather than "Type".

>+// Copyright (C) 2010-2019 Free Software Foundation, Inc.

Just 2019 for the date. It's a new file.
JeanHeyd Meneide Dec. 4, 2019, 10:56 p.m. UTC | #2 
Dear Jonathan,

    Done! re-patched, below.

Best,
JeanHeyd

2019-12-03  JeanHeyd "ThePhD" Meneide  <phdofthehouse@gmail.com>

        * include/bits/c++config: Add new _GLIBCXX20_DEPRECATED macro.
        * include/std/type_traits: Deprecate is_pod with message.
        * testuite/20_util/is_pod/deprecated-2a.cc (new): test deprecation
diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 7ccfc5f199d..9e85fc2c9e8 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -78,6 +78,7 @@
 //   _GLIBCXX_USE_DEPRECATED
 //   _GLIBCXX_DEPRECATED
 //   _GLIBCXX17_DEPRECATED
+//   _GLIBCXX20_DEPRECATED( STRINGS... )
 #ifndef _GLIBCXX_USE_DEPRECATED
 # define _GLIBCXX_USE_DEPRECATED 1
 #endif
@@ -94,6 +95,12 @@
 # define _GLIBCXX17_DEPRECATED
 #endif
 
+#if defined(__DEPRECATED) && (__cplusplus > 201703L)
+# define _GLIBCXX20_DEPRECATED(MSG) [[deprecated(MSG)]]
+#else
+# define _GLIBCXX20_DEPRECATED(MSG) 
+#endif
+
 // Macros for ABI tag attributes.
 #ifndef _GLIBCXX_ABI_TAG_CXX11
 # define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 8e787a994c3..5980b21c763 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -685,10 +685,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	"template argument must be a complete class or an unbounded array");
     };
 
-  /// is_pod
+  /// is_pod (deprecated C++2a)
   // Could use is_standard_layout && is_trivial instead of the builtin.
   template<typename _Tp>
-    struct is_pod
+    struct
+    _GLIBCXX20_DEPRECATED("is_pod<T> is deprecated in C++20: use (is_standard_layout_v<T> && is_trivial_v<T>) instead") 
+    is_pod
     : public integral_constant<bool, __is_pod(_Tp)>
     {
       static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
@@ -3073,6 +3075,7 @@ template <typename _Tp>
 template <typename _Tp>
   inline constexpr bool is_standard_layout_v = is_standard_layout<_Tp>::value;
 template <typename _Tp>
+  _GLIBCXX20_DEPRECATED("is_pod_v<T> is deprecated in C++20: use (is_standard_layout_v<T> && is_trivial_v<T>) instead")
   inline constexpr bool is_pod_v = is_pod<_Tp>::value;
 template <typename _Tp>
   inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
diff --git a/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
new file mode 100644
index 00000000000..1035a729434
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
@@ -0,0 +1,25 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++2a } }
+
+// { dg-prune-output "declared here" }
+
+#include <type_traits>
+
+static_assert(std::is_pod<int>::value); // { dg-warning "is deprecated" }
+static_assert(std::is_pod_v<int>); // { dg-warning "is deprecated" }
Jonathan Wakely Jan. 9, 2020, 7:57 p.m. UTC | #3 
On 04/12/19 17:56 -0500, JeanHeyd Meneide wrote:
>Dear Jonathan,
>
>    Done! re-patched, below.

Thanks!

Some comments below.

>Best,
>JeanHeyd
>
>2019-12-03  JeanHeyd "ThePhD" Meneide  <phdofthehouse@gmail.com>
>
>        * include/bits/c++config: Add new _GLIBCXX20_DEPRECATED macro.
>        * include/std/type_traits: Deprecate is_pod with message.
>        * testuite/20_util/is_pod/deprecated-2a.cc (new): test deprecation

The parenthesis after the filename should tell you the entity that was
changed, so something like:

	* include/bits/c++config (_GLIBCXX20_DEPRECATED): Add new macro.
	* include/std/type_traits (is_pod, is_pod_v): Deprecate for C++20.
	* testuite/20_util/is_pod/deprecated-2a.cc: New test.

This tells you that _GLIBCXX20_DEPRECATED is the new macro, and that
is_pod and is_pod_v were deprecated.

>diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
>index 7ccfc5f199d..9e85fc2c9e8 100644
>--- a/libstdc++-v3/include/bits/c++config
>+++ b/libstdc++-v3/include/bits/c++config
>@@ -78,6 +78,7 @@
> //   _GLIBCXX_USE_DEPRECATED
> //   _GLIBCXX_DEPRECATED
> //   _GLIBCXX17_DEPRECATED
>+//   _GLIBCXX20_DEPRECATED( STRINGS... )
> #ifndef _GLIBCXX_USE_DEPRECATED
> # define _GLIBCXX_USE_DEPRECATED 1
> #endif
>@@ -94,6 +95,12 @@
> # define _GLIBCXX17_DEPRECATED
> #endif
>
>+#if defined(__DEPRECATED) && (__cplusplus > 201703L)
>+# define _GLIBCXX20_DEPRECATED(MSG) [[deprecated(MSG)]]
>+#else
>+# define _GLIBCXX20_DEPRECATED(MSG)
>+#endif
>+
> // Macros for ABI tag attributes.
> #ifndef _GLIBCXX_ABI_TAG_CXX11
> # define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
>diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
>index 8e787a994c3..5980b21c763 100644
>--- a/libstdc++-v3/include/std/type_traits
>+++ b/libstdc++-v3/include/std/type_traits
>@@ -685,10 +685,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	"template argument must be a complete class or an unbounded array");
>     };
>
>-  /// is_pod
>+  /// is_pod (deprecated C++2a)
>   // Could use is_standard_layout && is_trivial instead of the builtin.
>   template<typename _Tp>
>-    struct is_pod
>+    struct
>+    _GLIBCXX20_DEPRECATED("is_pod<T> is deprecated in C++20: use (is_standard_layout_v<T> && is_trivial_v<T>) instead")
>+    is_pod
>     : public integral_constant<bool, __is_pod(_Tp)>
>     {
>       static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
>@@ -3073,6 +3075,7 @@ template <typename _Tp>
> template <typename _Tp>
>   inline constexpr bool is_standard_layout_v = is_standard_layout<_Tp>::value;
> template <typename _Tp>
>+  _GLIBCXX20_DEPRECATED("is_pod_v<T> is deprecated in C++20: use (is_standard_layout_v<T> && is_trivial_v<T>) instead")

This message ends up repeating the "is deprecated":

pod.cc:6:21: warning: 'std::is_pod_v<A>' is deprecated: is_pod_v<T> is deprecated in C++20: use (is_standard_layout_v<T> && is_trivial_v<T>) instead [-Wdeprecated-declarations]

I think we can just shorten it to the "use ... instead". Also, given
that the compiler prints the template argument list "<A>" it's a bit
confusing to also put argument lists (with different types) in the
message. I think this is better:

pod.cc:6:21: warning: 'std::is_pod_v<A>' is deprecated: use is_standard_layout_v && is_trivial_v instead [-Wdeprecated-declarations]


>   inline constexpr bool is_pod_v = is_pod<_Tp>::value;
> template <typename _Tp>
>   inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
>diff --git a/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
>new file mode 100644
>index 00000000000..1035a729434
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
>@@ -0,0 +1,25 @@
>+// Copyright (C) 2019 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+//
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+// GNU General Public License for more details.
>+//
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-do compile { target c++2a } }

This test doesn't actually run:

UNSUPPORTED: 20_util/is_pod/deprecated-2a.cc

It needs { dg-options "-std=gnu++2a" }

>+
>+// { dg-prune-output "declared here" }
>+
>+#include <type_traits>
>+
>+static_assert(std::is_pod<int>::value); // { dg-warning "is deprecated" }
>+static_assert(std::is_pod_v<int>); // { dg-warning "is deprecated" }

I'll commit the attached patch after more testing.

N.B. it looks like the implementation of [[deprecated(msg)]] isn't
working quite right, because for is_pod I don't get the message:

pod.cc:6:21: warning: 'template<class _Tp> struct std::is_pod' is deprecated [-Wdeprecated-declarations]
     6 | static_assert( std::is_pod<A>::value );
       |                     ^~~~~~
In file included from pod.cc:1:
/home/jwakely/gcc/10/include/c++/10.0.0/type_traits:693:5: note: declared here
   693 |     is_pod
       |     ^~~~~~

It works correctly for std::is_pod_v.
Jonathan Wakely Jan. 9, 2020, 9:20 p.m. UTC | #4 
On 09/01/20 19:57 +0000, Jonathan Wakely wrote:
>I'll commit the attached patch after more testing.

And this follow-up to fix some fallout.
Christophe Lyon Jan. 10, 2020, 12:25 p.m. UTC | #5 
Hi,

On Thu, 9 Jan 2020 at 22:21, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On 09/01/20 19:57 +0000, Jonathan Wakely wrote:
> >I'll commit the attached patch after more testing.
>
> And this follow-up to fix some fallout.
>

I have noticed:
FAIL: g++:g++.dg/cpp0x/std-layout1.C  -std=c++2a (test for excess errors)

was that follow-up intended to fix this too?
Jonathan Wakely Jan. 10, 2020, 1:55 p.m. UTC | #6 
On 10/01/20 13:25 +0100, Christophe Lyon wrote:
>Hi,
>
>On Thu, 9 Jan 2020 at 22:21, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> On 09/01/20 19:57 +0000, Jonathan Wakely wrote:
>> >I'll commit the attached patch after more testing.
>>
>> And this follow-up to fix some fallout.
>>
>
>I have noticed:
>FAIL: g++:g++.dg/cpp0x/std-layout1.C  -std=c++2a (test for excess errors)
>
>was that follow-up intended to fix this too?

No, I didn't touch the gcc tests.

This fixes it.

Jason, OK for trunk?
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config
index 7ccfc5f199d..5876b0b977b 100644
--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -78,6 +78,7 @@ 
 //   _GLIBCXX_USE_DEPRECATED
 //   _GLIBCXX_DEPRECATED
 //   _GLIBCXX17_DEPRECATED
+//   _GLIBCXX20_DEPRECATED( STRINGS... )
 #ifndef _GLIBCXX_USE_DEPRECATED
 # define _GLIBCXX_USE_DEPRECATED 1
 #endif
@@ -94,6 +95,12 @@ 
 # define _GLIBCXX17_DEPRECATED
 #endif
 
+#if defined(__DEPRECATED) && (__cplusplus > 201703L)
+# define _GLIBCXX20_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]
+#else
+# define _GLIBCXX20_DEPRECATED(...) 
+#endif
+
 // Macros for ABI tag attributes.
 #ifndef _GLIBCXX_ABI_TAG_CXX11
 # define _GLIBCXX_ABI_TAG_CXX11 __attribute ((__abi_tag__ ("cxx11")))
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index 8e787a994c3..91269d1bd02 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -685,10 +685,12 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	"template argument must be a complete class or an unbounded array");
     };
 
-  /// is_pod
+  /// is_pod (deprecated C++2a)
   // Could use is_standard_layout && is_trivial instead of the builtin.
   template<typename _Tp>
-    struct is_pod
+    struct
+    _GLIBCXX20_DEPRECATED("is_pod<Type> is deprecated in C++20: use is_standard_layout<Type> && is_trivial<Type> instead") 
+    is_pod
     : public integral_constant<bool, __is_pod(_Tp)>
     {
       static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
@@ -3073,6 +3075,7 @@  template <typename _Tp>
 template <typename _Tp>
   inline constexpr bool is_standard_layout_v = is_standard_layout<_Tp>::value;
 template <typename _Tp>
+  _GLIBCXX20_DEPRECATED("is_pod<Type> is deprecated in C++20: use is_standard_layout<Type> && is_trivial<Type> instead")
   inline constexpr bool is_pod_v = is_pod<_Tp>::value;
 template <typename _Tp>
   inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
diff --git a/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
new file mode 100644
index 00000000000..9782c3f551d
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_pod/deprecated-2a.cc
@@ -0,0 +1,25 @@ 
+// Copyright (C) 2010-2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++2a } }
+
+// { dg-prune-output "declared here" }
+
+#include <type_traits>
+
+static_assert(std::is_pod<int>::value); // { dg-warning "is deprecated" }
+static_assert(std::is_pod_v<int>); // { dg-warning "is deprecated" }