* [GSoC] Conflicted Built-in Trait Name @ 2023-03-25 7:53 Ken Matsui 2023-03-25 12:23 ` Roy Jacobson 2023-03-25 12:38 ` Marc Glisse 0 siblings, 2 replies; 18+ messages in thread From: Ken Matsui @ 2023-03-25 7:53 UTC (permalink / raw) To: gcc; +Cc: libstdc++ Hi, I am working on the GSoC project, "C++: Implement compiler built-in traits for the standard library traits". Built-in trait naming simply adds two underscores (__) to the original trait name. However, the same names are already in use for some built-in traits, such as is_void, is_pointer, and is_signed. For example, __is_void is used in the following files: * gcc/testsuite/g++.dg/tm/pr46567.C * libstdc++-v3/include/bits/cpp_type_traits.h In this case, are we supposed to change the existing same name in the code to something like ____is_void (four underscores)? Or is it better to break the naming convention of built-in traits like __is_void_builtin? Sincerely, Ken Matsui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-25 7:53 [GSoC] Conflicted Built-in Trait Name Ken Matsui @ 2023-03-25 12:23 ` Roy Jacobson 2023-03-27 21:58 ` Jonathan Wakely 2023-03-25 12:38 ` Marc Glisse 1 sibling, 1 reply; 18+ messages in thread From: Roy Jacobson @ 2023-03-25 12:23 UTC (permalink / raw) To: Ken Matsui; +Cc: gcc, libstdc++ [-- Attachment #1: Type: text/plain, Size: 1100 bytes --] Clang has been providing __is_void for a very long time now, and is definitely compatible with libstdc++. Does defining this builtin cause a problem? Might be that the lookup rules for builtins are different or something. https://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives On Sat, 25 Mar 2023 at 15:08, Ken Matsui via Gcc <gcc@gcc.gnu.org> wrote: > Hi, > > I am working on the GSoC project, "C++: Implement compiler built-in > traits for the standard library traits". > > Built-in trait naming simply adds two underscores (__) to the original > trait name. However, the same names are already in use for some > built-in traits, such as is_void, is_pointer, and is_signed. > > For example, __is_void is used in the following files: > > * gcc/testsuite/g++.dg/tm/pr46567.C > * libstdc++-v3/include/bits/cpp_type_traits.h > > In this case, are we supposed to change the existing same name in the > code to something like ____is_void (four underscores)? Or is it better > to break the naming convention of built-in traits like > __is_void_builtin? > > Sincerely, > Ken Matsui > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-25 12:23 ` Roy Jacobson @ 2023-03-27 21:58 ` Jonathan Wakely 2023-03-27 22:48 ` Ken Matsui 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Wakely @ 2023-03-27 21:58 UTC (permalink / raw) To: Roy Jacobson; +Cc: Ken Matsui, gcc, libstdc++ On Sat, 25 Mar 2023 at 12:23, Roy Jacobson via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Clang has been providing __is_void for a very long time now, and is > definitely compatible with libstdc++. Does defining this builtin cause a > problem? Might be that the lookup rules for builtins are different or > something. > > https://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives Clang has special hacks that allow it to handle libstdc++ headers. When it sees the use of a built-in trait like __is_void used as an identifier, it disables the built-in and treats it as a normal identifier for the rest of the translation unit. GCC doesn't do that, so "it compiles with Clang" isn't really relevant here. > > On Sat, 25 Mar 2023 at 15:08, Ken Matsui via Gcc <gcc@gcc.gnu.org> wrote: > > > Hi, > > > > I am working on the GSoC project, "C++: Implement compiler built-in > > traits for the standard library traits". > > > > Built-in trait naming simply adds two underscores (__) to the original > > trait name. However, the same names are already in use for some > > built-in traits, such as is_void, is_pointer, and is_signed. > > > > For example, __is_void is used in the following files: > > > > * gcc/testsuite/g++.dg/tm/pr46567.C > > * libstdc++-v3/include/bits/cpp_type_traits.h > > > > In this case, are we supposed to change the existing same name in the > > code to something like ____is_void (four underscores)? Or is it better > > to break the naming convention of built-in traits like > > __is_void_builtin? > > > > Sincerely, > > Ken Matsui > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 21:58 ` Jonathan Wakely @ 2023-03-27 22:48 ` Ken Matsui 0 siblings, 0 replies; 18+ messages in thread From: Ken Matsui @ 2023-03-27 22:48 UTC (permalink / raw) To: Roy Jacobson; +Cc: Jonathan Wakely, gcc, libstdc++ Jacobson's email was treated as spam somehow. Sorry for missing your email. On Mon, Mar 27, 2023 at 2:59 PM Jonathan Wakely <jwakely@redhat.com> wrote: > > On Sat, 25 Mar 2023 at 12:23, Roy Jacobson via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > Clang has been providing __is_void for a very long time now, and is > > definitely compatible with libstdc++. Does defining this builtin cause a > > problem? Might be that the lookup rules for builtins are different or > > something. > > > > https://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives > > Clang has special hacks that allow it to handle libstdc++ headers. > When it sees the use of a built-in trait like __is_void used as an > identifier, it disables the built-in and treats it as a normal > identifier for the rest of the translation unit. GCC doesn't do that, > so "it compiles with Clang" isn't really relevant here. > > > > > > On Sat, 25 Mar 2023 at 15:08, Ken Matsui via Gcc <gcc@gcc.gnu.org> wrote: > > > > > Hi, > > > > > > I am working on the GSoC project, "C++: Implement compiler built-in > > > traits for the standard library traits". > > > > > > Built-in trait naming simply adds two underscores (__) to the original > > > trait name. However, the same names are already in use for some > > > built-in traits, such as is_void, is_pointer, and is_signed. > > > > > > For example, __is_void is used in the following files: > > > > > > * gcc/testsuite/g++.dg/tm/pr46567.C > > > * libstdc++-v3/include/bits/cpp_type_traits.h > > > > > > In this case, are we supposed to change the existing same name in the > > > code to something like ____is_void (four underscores)? Or is it better > > > to break the naming convention of built-in traits like > > > __is_void_builtin? > > > > > > Sincerely, > > > Ken Matsui > > > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-25 7:53 [GSoC] Conflicted Built-in Trait Name Ken Matsui 2023-03-25 12:23 ` Roy Jacobson @ 2023-03-25 12:38 ` Marc Glisse 2023-03-26 2:01 ` Ken Matsui 1 sibling, 1 reply; 18+ messages in thread From: Marc Glisse @ 2023-03-25 12:38 UTC (permalink / raw) To: Ken Matsui; +Cc: gcc, libstdc++ On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > Built-in trait naming simply adds two underscores (__) to the original > trait name. However, the same names are already in use for some > built-in traits, such as is_void, is_pointer, and is_signed. > > For example, __is_void is used in the following files: > > * gcc/testsuite/g++.dg/tm/pr46567.C This is a testcase, you can rename __is_void to whatever in there, it doesn't matter. > * libstdc++-v3/include/bits/cpp_type_traits.h This __is_void seems to be used in a single place in include/debug/helper_functions.h, couldn't we tweak that code so __is_void becomes unused and can be removed? -- Marc Glisse ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-25 12:38 ` Marc Glisse @ 2023-03-26 2:01 ` Ken Matsui 2023-03-27 17:33 ` François Dumont 0 siblings, 1 reply; 18+ messages in thread From: Ken Matsui @ 2023-03-26 2:01 UTC (permalink / raw) To: libstdc++; +Cc: gcc On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > > On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > > > Built-in trait naming simply adds two underscores (__) to the original > > trait name. However, the same names are already in use for some > > built-in traits, such as is_void, is_pointer, and is_signed. > > > > For example, __is_void is used in the following files: > > > > * gcc/testsuite/g++.dg/tm/pr46567.C > > This is a testcase, you can rename __is_void to whatever in there, it > doesn't matter. > > > * libstdc++-v3/include/bits/cpp_type_traits.h > > This __is_void seems to be used in a single place in > include/debug/helper_functions.h, couldn't we tweak that code so __is_void > becomes unused and can be removed? That worked. Thank you! So, we can remove a code in a header as long as it is not standard and is not used elsewhere, can't we? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-26 2:01 ` Ken Matsui @ 2023-03-27 17:33 ` François Dumont 2023-03-27 20:16 ` Ken Matsui 2023-03-28 21:29 ` Ken Matsui 0 siblings, 2 replies; 18+ messages in thread From: François Dumont @ 2023-03-27 17:33 UTC (permalink / raw) To: Ken Matsui, libstdc++; +Cc: gcc On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: >> >>> Built-in trait naming simply adds two underscores (__) to the original >>> trait name. However, the same names are already in use for some >>> built-in traits, such as is_void, is_pointer, and is_signed. >>> >>> For example, __is_void is used in the following files: >>> >>> * gcc/testsuite/g++.dg/tm/pr46567.C >> This is a testcase, you can rename __is_void to whatever in there, it >> doesn't matter. >> >>> * libstdc++-v3/include/bits/cpp_type_traits.h >> This __is_void seems to be used in a single place in >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void >> becomes unused and can be removed? > That worked. Thank you! What worked ? > > So, we can remove a code in a header as long as it is not standard and > is not used elsewhere, can't we? You can do anything you like as long as you run the testsuite before presenting your patch. Here note that you'll need to run: make check-debug to run tests in _GLIBCXX_DEBUG mode which is making use of the code in helper_functions.h. Clearly this usage of std::__is_void could be replaced with your builtin by reimplementing _Distance_traits like this: template<typename _Iterator, typename = typename std::__is_integer<_Iterator>::__type> struct _Distance_traits { private: typedef typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; typedef typename std::conditional<__is_void<_ItDiffType>, std::ptrdiff_t, _ItDiffType>::type _DiffType; public: typedef std::pair<_DiffType, _Distance_precision> __type; }; this is untested, just to give you an idea of what your patch could be. François ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 17:33 ` François Dumont @ 2023-03-27 20:16 ` Ken Matsui 2023-03-27 21:43 ` Jonathan Wakely 2023-03-28 21:29 ` Ken Matsui 1 sibling, 1 reply; 18+ messages in thread From: Ken Matsui @ 2023-03-27 20:16 UTC (permalink / raw) To: François Dumont; +Cc: gcc, libstdc++ On Mon, Mar 27, 2023 at 10:33 AM François Dumont <frs.dumont@gmail.com> wrote: > > > On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > >> > >>> Built-in trait naming simply adds two underscores (__) to the original > >>> trait name. However, the same names are already in use for some > >>> built-in traits, such as is_void, is_pointer, and is_signed. > >>> > >>> For example, __is_void is used in the following files: > >>> > >>> * gcc/testsuite/g++.dg/tm/pr46567.C > >> This is a testcase, you can rename __is_void to whatever in there, it > >> doesn't matter. > >> > >>> * libstdc++-v3/include/bits/cpp_type_traits.h > >> This __is_void seems to be used in a single place in > >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void > >> becomes unused and can be removed? > > That worked. Thank you! > What worked ? If you meant about the code, I sent a patch series, as you can see in the following link. https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614620.html * In the following patch, I replaced __is_void with std::is_void in helper_functions.h. https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614624.html * And then, I deleted __is_void in cpp_type_traits.h. https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614623.html * Also, I replaced __is_void with ____is_void in the testcase. https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614625.html If you meant what tests had been done, I did the following tests because the header is under libstdc++. * make check-gcc-c++ (because the patches involve changes on GCC) * make check-target-libstdc++-v3 > > So, we can remove a code in a header as long as it is not standard and > > is not used elsewhere, can't we? > > You can do anything you like as long as you run the testsuite before > presenting your patch. Here note that you'll need to run: > > make check-debug > > to run tests in _GLIBCXX_DEBUG mode which is making use of the code in > helper_functions.h. > > Clearly this usage of std::__is_void could be replaced with your builtin > by reimplementing _Distance_traits like this: > > template<typename _Iterator, > typename = typename std::__is_integer<_Iterator>::__type> > struct _Distance_traits > { > private: > typedef > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > typedef > typename std::conditional<__is_void<_ItDiffType>, > std::ptrdiff_t, _ItDiffType>::type _DiffType; > > public: > typedef std::pair<_DiffType, _Distance_precision> __type; > }; > > this is untested, just to give you an idea of what your patch could be. > > François Thank you! I thought the tests that I did included the hepler_function.h header, but they don’t...? If not, I must check if std::is_void in the type_traits header was correctly used. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 20:16 ` Ken Matsui @ 2023-03-27 21:43 ` Jonathan Wakely 2023-03-27 21:49 ` Jonathan Wakely 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Wakely @ 2023-03-27 21:43 UTC (permalink / raw) To: Ken Matsui; +Cc: François Dumont, gcc, libstdc++ On Mon, 27 Mar 2023 at 21:17, Ken Matsui via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > On Mon, Mar 27, 2023 at 10:33 AM François Dumont <frs.dumont@gmail.com> wrote: > > > > > > On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > > > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > > >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > > >> > > >>> Built-in trait naming simply adds two underscores (__) to the original > > >>> trait name. However, the same names are already in use for some > > >>> built-in traits, such as is_void, is_pointer, and is_signed. > > >>> > > >>> For example, __is_void is used in the following files: > > >>> > > >>> * gcc/testsuite/g++.dg/tm/pr46567.C > > >> This is a testcase, you can rename __is_void to whatever in there, it > > >> doesn't matter. > > >> > > >>> * libstdc++-v3/include/bits/cpp_type_traits.h > > >> This __is_void seems to be used in a single place in > > >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void > > >> becomes unused and can be removed? > > > That worked. Thank you! > > What worked ? > > If you meant about the code, I sent a patch series, as you can see in > the following link. > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614620.html > > * In the following patch, I replaced __is_void with std::is_void in > helper_functions.h. You can't do that. std::is_void is not present in C++98 mode, but the code in <debug/helper_functions.h> needs to work for C++98 mode. > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614624.html > * And then, I deleted __is_void in cpp_type_traits.h. > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614623.html > * Also, I replaced __is_void with ____is_void in the testcase. > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614625.html > > If you meant what tests had been done, I did the following tests > because the header is under libstdc++. > > * make check-gcc-c++ (because the patches involve changes on GCC) > * make check-target-libstdc++-v3 > > > > So, we can remove a code in a header as long as it is not standard and > > > is not used elsewhere, can't we? > > > > You can do anything you like as long as you run the testsuite before > > presenting your patch. Here note that you'll need to run: > > > > make check-debug > > > > to run tests in _GLIBCXX_DEBUG mode which is making use of the code in > > helper_functions.h. > > > > Clearly this usage of std::__is_void could be replaced with your builtin > > by reimplementing _Distance_traits like this: > > > > template<typename _Iterator, > > typename = typename std::__is_integer<_Iterator>::__type> > > struct _Distance_traits > > { > > private: > > typedef > > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > > > typedef > > typename std::conditional<__is_void<_ItDiffType>, > > std::ptrdiff_t, _ItDiffType>::type _DiffType; > > > > public: > > typedef std::pair<_DiffType, _Distance_precision> __type; > > }; > > > > this is untested, just to give you an idea of what your patch could be. > > > > François > > Thank you! I thought the tests that I did included the > hepler_function.h header, but they don’t...? If not, I must check if > std::is_void in the type_traits header was correctly used. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 21:43 ` Jonathan Wakely @ 2023-03-27 21:49 ` Jonathan Wakely 2023-03-27 21:55 ` Ken Matsui 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Wakely @ 2023-03-27 21:49 UTC (permalink / raw) To: Ken Matsui; +Cc: François Dumont, gcc, libstdc++ On Mon, 27 Mar 2023 at 22:43, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > On Mon, 27 Mar 2023 at 21:17, Ken Matsui via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > On Mon, Mar 27, 2023 at 10:33 AM François Dumont <frs.dumont@gmail.com> wrote: > > > > > > > > > On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > > > > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > > > >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > > > >> > > > >>> Built-in trait naming simply adds two underscores (__) to the original > > > >>> trait name. However, the same names are already in use for some > > > >>> built-in traits, such as is_void, is_pointer, and is_signed. > > > >>> > > > >>> For example, __is_void is used in the following files: > > > >>> > > > >>> * gcc/testsuite/g++.dg/tm/pr46567.C > > > >> This is a testcase, you can rename __is_void to whatever in there, it > > > >> doesn't matter. > > > >> > > > >>> * libstdc++-v3/include/bits/cpp_type_traits.h > > > >> This __is_void seems to be used in a single place in > > > >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void > > > >> becomes unused and can be removed? > > > > That worked. Thank you! > > > What worked ? > > > > If you meant about the code, I sent a patch series, as you can see in > > the following link. > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614620.html > > > > * In the following patch, I replaced __is_void with std::is_void in > > helper_functions.h. > > You can't do that. std::is_void is not present in C++98 mode, but the > code in <debug/helper_functions.h> needs to work for C++98 mode. > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614624.html > > * And then, I deleted __is_void in cpp_type_traits.h. > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614623.html > > * Also, I replaced __is_void with ____is_void in the testcase. > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614625.html > > > > If you meant what tests had been done, I did the following tests > > because the header is under libstdc++. > > > > * make check-gcc-c++ (because the patches involve changes on GCC) > > * make check-target-libstdc++-v3 > > > > > > So, we can remove a code in a header as long as it is not standard and > > > > is not used elsewhere, can't we? > > > > > > You can do anything you like as long as you run the testsuite before > > > presenting your patch. Here note that you'll need to run: > > > > > > make check-debug > > > > > > to run tests in _GLIBCXX_DEBUG mode which is making use of the code in > > > helper_functions.h. > > > > > > Clearly this usage of std::__is_void could be replaced with your builtin > > > by reimplementing _Distance_traits like this: > > > > > > template<typename _Iterator, > > > typename = typename std::__is_integer<_Iterator>::__type> > > > struct _Distance_traits > > > { > > > private: > > > typedef > > > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > > > > > typedef > > > typename std::conditional<__is_void<_ItDiffType>, > > > std::ptrdiff_t, _ItDiffType>::type _DiffType; > > > > > > public: > > > typedef std::pair<_DiffType, _Distance_precision> __type; > > > }; > > > > > > this is untested, just to give you an idea of what your patch could be. > > > > > > François > > > > Thank you! I thought the tests that I did included the > > hepler_function.h header, but they don’t...? If not, I must check if > > std::is_void in the type_traits header was correctly used. Something like this would work: --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -66,13 +66,12 @@ namespace __gnu_debug typedef typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; - template<typename _DiffType, - typename = typename std::__is_void<_DiffType>::__type> + template<typename _DiffType, typename = const _DiffType> struct _DiffTraits { typedef _DiffType __type; }; template<typename _DiffType> - struct _DiffTraits<_DiffType, std::__true_type> + struct _DiffTraits<_DiffType, const void> { typedef std::ptrdiff_t __type; }; typedef typename _DiffTraits<_ItDiffType>::__type _DiffType; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 21:49 ` Jonathan Wakely @ 2023-03-27 21:55 ` Ken Matsui 0 siblings, 0 replies; 18+ messages in thread From: Ken Matsui @ 2023-03-27 21:55 UTC (permalink / raw) To: Jonathan Wakely; +Cc: François Dumont, gcc, libstdc++ Oh! Thank you! On Mon, Mar 27, 2023 at 2:49 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > On Mon, 27 Mar 2023 at 22:43, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > > > > On Mon, 27 Mar 2023 at 21:17, Ken Matsui via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > > > > > > On Mon, Mar 27, 2023 at 10:33 AM François Dumont <frs.dumont@gmail.com> wrote: > > > > > > > > > > > > On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > > > > > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > > > > >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > > > > >> > > > > >>> Built-in trait naming simply adds two underscores (__) to the original > > > > >>> trait name. However, the same names are already in use for some > > > > >>> built-in traits, such as is_void, is_pointer, and is_signed. > > > > >>> > > > > >>> For example, __is_void is used in the following files: > > > > >>> > > > > >>> * gcc/testsuite/g++.dg/tm/pr46567.C > > > > >> This is a testcase, you can rename __is_void to whatever in there, it > > > > >> doesn't matter. > > > > >> > > > > >>> * libstdc++-v3/include/bits/cpp_type_traits.h > > > > >> This __is_void seems to be used in a single place in > > > > >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void > > > > >> becomes unused and can be removed? > > > > > That worked. Thank you! > > > > What worked ? > > > > > > If you meant about the code, I sent a patch series, as you can see in > > > the following link. > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614620.html > > > > > > * In the following patch, I replaced __is_void with std::is_void in > > > helper_functions.h. > > > > You can't do that. std::is_void is not present in C++98 mode, but the > > code in <debug/helper_functions.h> needs to work for C++98 mode. > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614624.html > > > * And then, I deleted __is_void in cpp_type_traits.h. > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614623.html > > > * Also, I replaced __is_void with ____is_void in the testcase. > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614625.html > > > > > > If you meant what tests had been done, I did the following tests > > > because the header is under libstdc++. > > > > > > * make check-gcc-c++ (because the patches involve changes on GCC) > > > * make check-target-libstdc++-v3 > > > > > > > > So, we can remove a code in a header as long as it is not standard and > > > > > is not used elsewhere, can't we? > > > > > > > > You can do anything you like as long as you run the testsuite before > > > > presenting your patch. Here note that you'll need to run: > > > > > > > > make check-debug > > > > > > > > to run tests in _GLIBCXX_DEBUG mode which is making use of the code in > > > > helper_functions.h. > > > > > > > > Clearly this usage of std::__is_void could be replaced with your builtin > > > > by reimplementing _Distance_traits like this: > > > > > > > > template<typename _Iterator, > > > > typename = typename std::__is_integer<_Iterator>::__type> > > > > struct _Distance_traits > > > > { > > > > private: > > > > typedef > > > > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > > > > > > > typedef > > > > typename std::conditional<__is_void<_ItDiffType>, > > > > std::ptrdiff_t, _ItDiffType>::type _DiffType; > > > > > > > > public: > > > > typedef std::pair<_DiffType, _Distance_precision> __type; > > > > }; > > > > > > > > this is untested, just to give you an idea of what your patch could be. > > > > > > > > François > > > > > > Thank you! I thought the tests that I did included the > > > hepler_function.h header, but they don’t...? If not, I must check if > > > std::is_void in the type_traits header was correctly used. > > Something like this would work: > > --- a/libstdc++-v3/include/debug/helper_functions.h > +++ b/libstdc++-v3/include/debug/helper_functions.h > @@ -66,13 +66,12 @@ namespace __gnu_debug > typedef > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > - template<typename _DiffType, > - typename = typename std::__is_void<_DiffType>::__type> > + template<typename _DiffType, typename = const _DiffType> > struct _DiffTraits > { typedef _DiffType __type; }; > > template<typename _DiffType> > - struct _DiffTraits<_DiffType, std::__true_type> > + struct _DiffTraits<_DiffType, const void> > { typedef std::ptrdiff_t __type; }; > > typedef typename _DiffTraits<_ItDiffType>::__type _DiffType; ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-27 17:33 ` François Dumont 2023-03-27 20:16 ` Ken Matsui @ 2023-03-28 21:29 ` Ken Matsui 2023-03-28 23:24 ` Jonathan Wakely 1 sibling, 1 reply; 18+ messages in thread From: Ken Matsui @ 2023-03-28 21:29 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc Hi François, I tried to use `make check-debug`, but my Makefile does not include the target. Could you please tell me how you generated your Makefile? FYI, I did this command: `../configure --enable-languages=c++ --disable-error --disable-bootstrap`. Sincerely, Ken Matsui On Mon, Mar 27, 2023 at 10:33 AM François Dumont <frs.dumont@gmail.com> wrote: > > > On 26/03/2023 04:01, Ken Matsui via Libstdc++ wrote: > > On Sat, Mar 25, 2023 at 5:38 AM Marc Glisse <marc.glisse@inria.fr> wrote: > >> On Sat, 25 Mar 2023, Ken Matsui via Gcc wrote: > >> > >>> Built-in trait naming simply adds two underscores (__) to the original > >>> trait name. However, the same names are already in use for some > >>> built-in traits, such as is_void, is_pointer, and is_signed. > >>> > >>> For example, __is_void is used in the following files: > >>> > >>> * gcc/testsuite/g++.dg/tm/pr46567.C > >> This is a testcase, you can rename __is_void to whatever in there, it > >> doesn't matter. > >> > >>> * libstdc++-v3/include/bits/cpp_type_traits.h > >> This __is_void seems to be used in a single place in > >> include/debug/helper_functions.h, couldn't we tweak that code so __is_void > >> becomes unused and can be removed? > > That worked. Thank you! > What worked ? > > > > So, we can remove a code in a header as long as it is not standard and > > is not used elsewhere, can't we? > > You can do anything you like as long as you run the testsuite before > presenting your patch. Here note that you'll need to run: > > make check-debug > > to run tests in _GLIBCXX_DEBUG mode which is making use of the code in > helper_functions.h. > > Clearly this usage of std::__is_void could be replaced with your builtin > by reimplementing _Distance_traits like this: > > template<typename _Iterator, > typename = typename std::__is_integer<_Iterator>::__type> > struct _Distance_traits > { > private: > typedef > typename std::iterator_traits<_Iterator>::difference_type _ItDiffType; > > typedef > typename std::conditional<__is_void<_ItDiffType>, > std::ptrdiff_t, _ItDiffType>::type _DiffType; > > public: > typedef std::pair<_DiffType, _Distance_precision> __type; > }; > > this is untested, just to give you an idea of what your patch could be. > > François > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-28 21:29 ` Ken Matsui @ 2023-03-28 23:24 ` Jonathan Wakely 2023-03-28 23:33 ` Ken Matsui 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Wakely @ 2023-03-28 23:24 UTC (permalink / raw) To: Ken Matsui; +Cc: François Dumont, libstdc++, gcc On Tue, 28 Mar 2023 at 22:30, Ken Matsui via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Hi François, > > I tried to use `make check-debug`, but my Makefile does not include > the target. Could you please tell me how you generated your Makefile? It's a target in the libstdc++ makefile, so you need to run it from the $target/libstdc++-v3 directory. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-28 23:24 ` Jonathan Wakely @ 2023-03-28 23:33 ` Ken Matsui 2023-03-30 5:11 ` François Dumont 0 siblings, 1 reply; 18+ messages in thread From: Ken Matsui @ 2023-03-28 23:33 UTC (permalink / raw) To: Jonathan Wakely; +Cc: François Dumont, libstdc++, gcc Oooh, thank you for your help! On Tue, Mar 28, 2023 at 4:25 PM Jonathan Wakely <jwakely@redhat.com> wrote: > > On Tue, 28 Mar 2023 at 22:30, Ken Matsui via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: > > > > Hi François, > > > > I tried to use `make check-debug`, but my Makefile does not include > > the target. Could you please tell me how you generated your Makefile? > > It's a target in the libstdc++ makefile, so you need to run it from > the $target/libstdc++-v3 directory. > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-28 23:33 ` Ken Matsui @ 2023-03-30 5:11 ` François Dumont 2023-03-30 8:33 ` Ken Matsui 0 siblings, 1 reply; 18+ messages in thread From: François Dumont @ 2023-03-30 5:11 UTC (permalink / raw) To: Ken Matsui, Jonathan Wakely; +Cc: libstdc++, gcc Hi Do not hesitate to dig into library doc. Especially this page: https://gcc.gnu.org/onlinedocs/gcc-8.1.0/libstdc++/manual/manual/test.html You can also find it in your git clone in <gcc-repo>/libstdc++-v3/doc/html. You'll see also how to run test in different std modes like --std=c++98 to catch the kind of issue reported by Jonathan. Regarding your patches I wonder if it's not too splitted. 1 patch per builtin would sound more logical, at least for an easy one like __is_void. François On 29/03/2023 01:33, Ken Matsui wrote: > Oooh, thank you for your help! > > On Tue, Mar 28, 2023 at 4:25 PM Jonathan Wakely <jwakely@redhat.com> wrote: >> On Tue, 28 Mar 2023 at 22:30, Ken Matsui via Libstdc++ >> <libstdc++@gcc.gnu.org> wrote: >>> Hi François, >>> >>> I tried to use `make check-debug`, but my Makefile does not include >>> the target. Could you please tell me how you generated your Makefile? >> It's a target in the libstdc++ makefile, so you need to run it from >> the $target/libstdc++-v3 directory. >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-30 5:11 ` François Dumont @ 2023-03-30 8:33 ` Ken Matsui 2023-03-30 12:23 ` Jonathan Wakely 0 siblings, 1 reply; 18+ messages in thread From: Ken Matsui @ 2023-03-30 8:33 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc Hi François, On Wed, Mar 29, 2023 at 10:11 PM François Dumont <frs.dumont@gmail.com> wrote: > > Hi > > Do not hesitate to dig into library doc. Especially this page: > > https://gcc.gnu.org/onlinedocs/gcc-8.1.0/libstdc++/manual/manual/test.html > > You can also find it in your git clone in <gcc-repo>/libstdc++-v3/doc/html. > > You'll see also how to run test in different std modes like --std=c++98 > to catch the kind of issue reported by Jonathan. This is what I wanted to know! Thank you so much! > Regarding your patches I wonder if it's not too splitted. 1 patch per > builtin would sound more logical, at least for an easy one like __is_void. I see. I will squash is_void-related commits into the commit of __is_void implementation. Thank you for pointing it out! Sincerely, Ken Matsui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-30 8:33 ` Ken Matsui @ 2023-03-30 12:23 ` Jonathan Wakely 2023-03-30 18:44 ` Ken Matsui 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Wakely @ 2023-03-30 12:23 UTC (permalink / raw) To: Ken Matsui; +Cc: François Dumont, libstdc++, gcc On Thu, 30 Mar 2023 at 09:33, Ken Matsui wrote: > > Hi François, > > On Wed, Mar 29, 2023 at 10:11 PM François Dumont <frs.dumont@gmail.com> wrote: > > > > Hi > > > > Do not hesitate to dig into library doc. Especially this page: > > > > https://gcc.gnu.org/onlinedocs/gcc-8.1.0/libstdc++/manual/manual/test.html > > > > You can also find it in your git clone in <gcc-repo>/libstdc++-v3/doc/html. > > > > You'll see also how to run test in different std modes like --std=c++98 > > to catch the kind of issue reported by Jonathan. > > This is what I wanted to know! Thank you so much! > > > Regarding your patches I wonder if it's not too splitted. 1 patch per > > builtin would sound more logical, at least for an easy one like __is_void. > > I see. I will squash is_void-related commits into the commit of > __is_void implementation. Thank you for pointing it out! Yes, good point, François. These kind of changes for the front-end and library should be in one patch. Otherwise, if they were committed separately then you would create a revision where bootstrap fails. If the front-end change is committed without the library change, then you can't build the library because it still uses __is_void which is now a keyword. If the library change is committed first then you don't break bootstrap, but you're adding support to the library for a new built-in which doesn't actually exist (yet). They should be a single revision, so that the tree can always be built. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC] Conflicted Built-in Trait Name 2023-03-30 12:23 ` Jonathan Wakely @ 2023-03-30 18:44 ` Ken Matsui 0 siblings, 0 replies; 18+ messages in thread From: Ken Matsui @ 2023-03-30 18:44 UTC (permalink / raw) To: Jonathan Wakely; +Cc: François Dumont, libstdc++, gcc I see. Thank you for the clarification! On Thu, Mar 30, 2023 at 5:23 AM Jonathan Wakely <jwakely@redhat.com> wrote: > > On Thu, 30 Mar 2023 at 09:33, Ken Matsui wrote: > > > > Hi François, > > > > On Wed, Mar 29, 2023 at 10:11 PM François Dumont <frs.dumont@gmail.com> wrote: > > > > > > Hi > > > > > > Do not hesitate to dig into library doc. Especially this page: > > > > > > https://gcc.gnu.org/onlinedocs/gcc-8.1.0/libstdc++/manual/manual/test.html > > > > > > You can also find it in your git clone in <gcc-repo>/libstdc++-v3/doc/html. > > > > > > You'll see also how to run test in different std modes like --std=c++98 > > > to catch the kind of issue reported by Jonathan. > > > > This is what I wanted to know! Thank you so much! > > > > > Regarding your patches I wonder if it's not too splitted. 1 patch per > > > builtin would sound more logical, at least for an easy one like __is_void. > > > > I see. I will squash is_void-related commits into the commit of > > __is_void implementation. Thank you for pointing it out! > > > Yes, good point, François. These kind of changes for the front-end and > library should be in one patch. Otherwise, if they were committed > separately then you would create a revision where bootstrap fails. If > the front-end change is committed without the library change, then you > can't build the library because it still uses __is_void which is now a > keyword. If the library change is committed first then you don't break > bootstrap, but you're adding support to the library for a new built-in > which doesn't actually exist (yet). They should be a single revision, > so that the tree can always be built. > ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-03-30 18:44 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-25 7:53 [GSoC] Conflicted Built-in Trait Name Ken Matsui 2023-03-25 12:23 ` Roy Jacobson 2023-03-27 21:58 ` Jonathan Wakely 2023-03-27 22:48 ` Ken Matsui 2023-03-25 12:38 ` Marc Glisse 2023-03-26 2:01 ` Ken Matsui 2023-03-27 17:33 ` François Dumont 2023-03-27 20:16 ` Ken Matsui 2023-03-27 21:43 ` Jonathan Wakely 2023-03-27 21:49 ` Jonathan Wakely 2023-03-27 21:55 ` Ken Matsui 2023-03-28 21:29 ` Ken Matsui 2023-03-28 23:24 ` Jonathan Wakely 2023-03-28 23:33 ` Ken Matsui 2023-03-30 5:11 ` François Dumont 2023-03-30 8:33 ` Ken Matsui 2023-03-30 12:23 ` Jonathan Wakely 2023-03-30 18:44 ` Ken Matsui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).