| Message ID | 940535EF-38BA-4F5F-999A-2DE638B1B8F5@adacore.com |
|---|---|
| State | New |
| Headers | show |
| Series | rename local _C2 identifiers in stl map header files | expand |
On 09/01/20 14:00 +0100, Olivier Hainque wrote: >Hello, > >The attached patch is a proposal to simply rename a template >type name in stl_map.h and stl_multimap.h in order to prevent >clashes with a macro name exposed by a system header on VxWorks. > >The conflicting name is _C2, which in principle is "reserved" >for the system so having such a macro exposed by a system header >doesn't seem to be a bug per se. > >I suppose that the libstdc++ headers can be considered part of >the system as well in a way, so it's not really a bug there either. Any conflict with OS headers is treated as a libstdc++ bug. We don't own the entire implementation namespace, so we have to play nice and avoid OS names. We maintain a list of identifiers to avoid, but _C2 is currently missing. Could you please add it to the coding_style.bad_identifiers section in doc/xml/manual/appendix_contributing.xml so we know not to reintroduce _C2 in future? Thanks. >Fixing the OS headers is a major hassle on VxWorks, however, >especially with more recent versions of the system, so we arranged >not to rely on it at all up to now and a straightforward adjustment >in the libstdc++ headers offers a way smoother and simpler way >forward. > >I'm not sure what "C2" was intended to refer to so I picked It's the Comparison function for the container. Please use _Cmp2, which is consistent with the partial specialization defined at the end of the <bits/stl_map.h> header. >an alternate name which seemed to match some aspects of the context >while keeping the very-short-identifier style used throughout. > >This cures a lot of test failures with our gcc-9 based >toolchains for VwWorks 7 and applies cleanly on mainline. > >I'm running a bootstrap & regression test cycle on x86_64-linux > >Would this be OK to commit ? Yes, this is fine in principle. Please update the docs and change _Mt to _Cmp2 though.
Hello Jonathan, > On 09 Jan 2020, at 14:26, Jonathan Wakely <jwakely@redhat.com> wrote: > > Any conflict with OS headers is treated as a libstdc++ bug. We don't > own the entire implementation namespace, so we have to play nice and > avoid OS names. > We maintain a list of identifiers to avoid, but _C2 is currently > missing. Could you please add it to the coding_style.bad_identifiers > section in doc/xml/manual/appendix_contributing.xml so we know not to > reintroduce _C2 in future? Thanks. Sure. Thanks for explaining. This enlightens a lot for me :) >> Fixing the OS headers is a major hassle on VxWorks, however, >> especially with more recent versions of the system, so we arranged >> not to rely on it at all up to now and a straightforward adjustment >> in the libstdc++ headers offers a way smoother and simpler way >> forward. >> >> I'm not sure what "C2" was intended to refer to so I picked > > It's the Comparison function for the container. Please use _Cmp2, > which is consistent with the partial specialization defined at the end > of the <bits/stl_map.h> header. Ok, > Yes, this is fine in principle. Please update the docs and change > _Mt to _Cmp2 though. Sure, will adjust, test and follow up. Thanks a lot for your lightning fast constructive feedback. Best Regards, Olivier
Hi Jonathan, >> Yes, this is fine in principle. Please update the docs and change >> _Mt to _Cmp2 though. > > Sure, will adjust, test and follow up. Revised patch attached. Bootstrapped and regression tests fine on x86_64-linux. 2020-01-09 Olivier Hainque <hainque@adacore.com> libstdc++-v3/ * doc/xml/manual/appendix_contributing.xml: Document _C2 as a reserved identifier, by VxWorks. * include/bits/stl_map.h: Rename _C2 template typenames as _Cmp2. * include/bits/stl_multimap.h: Likewise. diff --git a/libstdc++-v3/doc/xml/manual/appendix_contributing.xml b/libstdc++-v3/doc/xml/manual/appendix_contributing.xml index 3e10e1b5e5c..b399e71d482 100644 --- a/libstdc++-v3/doc/xml/manual/appendix_contributing.xml +++ b/libstdc++-v3/doc/xml/manual/appendix_contributing.xml @@ -463,6 +463,9 @@ indicate a place that may require attention for multi-thread safety. _res_ext __tg_* + VxWorks adds: + _C2 + For GCC: [Note that this list is out of date. It applies to the old diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h index 4e4b82f5217..fe930c15757 100644 --- a/libstdc++-v3/include/bits/stl_map.h +++ b/libstdc++-v3/include/bits/stl_map.h @@ -637,30 +637,30 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template<typename, typename> friend class std::_Rb_tree_merge_helper; - template<typename _C2> + template<typename _Cmp2> void - merge(map<_Key, _Tp, _C2, _Alloc>& __source) + merge(map<_Key, _Tp, _Cmp2, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<map, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<map, _Cmp2>; _M_t._M_merge_unique(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Cmp2> void - merge(map<_Key, _Tp, _C2, _Alloc>&& __source) + merge(map<_Key, _Tp, _Cmp2, _Alloc>&& __source) { merge(__source); } - template<typename _C2> + template<typename _Cmp2> void - merge(multimap<_Key, _Tp, _C2, _Alloc>& __source) + merge(multimap<_Key, _Tp, _Cmp2, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<map, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<map, _Cmp2>; _M_t._M_merge_unique(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Cmp2> void - merge(multimap<_Key, _Tp, _C2, _Alloc>&& __source) + merge(multimap<_Key, _Tp, _Cmp2, _Alloc>&& __source) { merge(__source); } #endif // C++17 diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h index 48f57788798..d38f530e123 100644 --- a/libstdc++-v3/include/bits/stl_multimap.h +++ b/libstdc++-v3/include/bits/stl_multimap.h @@ -653,30 +653,30 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template<typename, typename> friend class std::_Rb_tree_merge_helper; - template<typename _C2> + template<typename _Cmp2> void - merge(multimap<_Key, _Tp, _C2, _Alloc>& __source) + merge(multimap<_Key, _Tp, _Cmp2, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<multimap, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<multimap, _Cmp2>; _M_t._M_merge_equal(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Cmp2> void - merge(multimap<_Key, _Tp, _C2, _Alloc>&& __source) + merge(multimap<_Key, _Tp, _Cmp2, _Alloc>&& __source) { merge(__source); } - template<typename _C2> + template<typename _Cmp2> void - merge(map<_Key, _Tp, _C2, _Alloc>& __source) + merge(map<_Key, _Tp, _Cmp2, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<multimap, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<multimap, _Cmp2>; _M_t._M_merge_equal(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Cmp2> void - merge(map<_Key, _Tp, _C2, _Alloc>&& __source) + merge(map<_Key, _Tp, _Cmp2, _Alloc>&& __source) { merge(__source); } #endif // C++17
On 09/01/20 20:09 +0100, Olivier Hainque wrote: >Hi Jonathan, > >>> Yes, this is fine in principle. Please update the docs and change >>> _Mt to _Cmp2 though. >> >> Sure, will adjust, test and follow up. > >Revised patch attached. Bootstrapped and regression tests >fine on x86_64-linux. > >2020-01-09 Olivier Hainque <hainque@adacore.com> > > libstdc++-v3/ > * doc/xml/manual/appendix_contributing.xml: Document _C2 > as a reserved identifier, by VxWorks. > * include/bits/stl_map.h: Rename _C2 template typenames as _Cmp2. > * include/bits/stl_multimap.h: Likewise. OK for trunk, thanks!
> On 9 Jan 2020, at 21:01, Jonathan Wakely <jwakely@redhat.com> wrote: > >> >> 2020-01-09 Olivier Hainque <hainque@adacore.com> >> >> libstdc++-v3/ >> * doc/xml/manual/appendix_contributing.xml: Document _C2 >> as a reserved identifier, by VxWorks. >> * include/bits/stl_map.h: Rename _C2 template typenames as _Cmp2. >> * include/bits/stl_multimap.h: Likewise. > > OK for trunk, thanks! Great, thanks :-)
diff --git a/libstdc++-v3/include/bits/stl_map.h b/libstdc++-v3/include/bits/stl_map.h index 322d0a8290a..08170b91f00 100644 --- a/libstdc++-v3/include/bits/stl_map.h +++ b/libstdc++-v3/include/bits/stl_map.h @@ -635,30 +635,30 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template<typename, typename> friend class std::_Rb_tree_merge_helper; - template<typename _C2> + template<typename _Mt> void - merge(map<_Key, _Tp, _C2, _Alloc>& __source) + merge(map<_Key, _Tp, _Mt, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<map, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<map, _Mt>; _M_t._M_merge_unique(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Mt> void - merge(map<_Key, _Tp, _C2, _Alloc>&& __source) + merge(map<_Key, _Tp, _Mt, _Alloc>&& __source) { merge(__source); } - template<typename _C2> + template<typename _Mt> void - merge(multimap<_Key, _Tp, _C2, _Alloc>& __source) + merge(multimap<_Key, _Tp, _Mt, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<map, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<map, _Mt>; _M_t._M_merge_unique(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Mt> void - merge(multimap<_Key, _Tp, _C2, _Alloc>&& __source) + merge(multimap<_Key, _Tp, _Mt, _Alloc>&& __source) { merge(__source); } #endif // C++17 diff --git a/libstdc++-v3/include/bits/stl_multimap.h b/libstdc++-v3/include/bits/stl_multimap.h index 4c4ccad3ac0..099f1c99cae 100644 --- a/libstdc++-v3/include/bits/stl_multimap.h +++ b/libstdc++-v3/include/bits/stl_multimap.h @@ -651,30 +651,30 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER template<typename, typename> friend class std::_Rb_tree_merge_helper; - template<typename _C2> + template<typename _Mt> void - merge(multimap<_Key, _Tp, _C2, _Alloc>& __source) + merge(multimap<_Key, _Tp, _Mt, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<multimap, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<multimap, _Mt>; _M_t._M_merge_equal(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Mt> void - merge(multimap<_Key, _Tp, _C2, _Alloc>&& __source) + merge(multimap<_Key, _Tp, _Mt, _Alloc>&& __source) { merge(__source); } - template<typename _C2> + template<typename _Mt> void - merge(map<_Key, _Tp, _C2, _Alloc>& __source) + merge(map<_Key, _Tp, _Mt, _Alloc>& __source) { - using _Merge_helper = _Rb_tree_merge_helper<multimap, _C2>; + using _Merge_helper = _Rb_tree_merge_helper<multimap, _Mt>; _M_t._M_merge_equal(_Merge_helper::_S_get_tree(__source)); } - template<typename _C2> + template<typename _Mt> void - merge(map<_Key, _Tp, _C2, _Alloc>&& __source) + merge(map<_Key, _Tp, _Mt, _Alloc>&& __source) { merge(__source); } #endif // C++17