https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127690
Bug ID: 127690
Summary: Problems with pb_ds sources
Product: gcc
Version: 17.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: redi at gcc dot gnu.org
Target Milestone: ---
The containers only support C++98 rules, so no move semantics, no allocator
propagation, no support for stateful allocators. This makes the containers
inflexible and less performant than they could be.
It doesn't use variadic templates, despite those being supported as an
extension in C++98 mode. So it depends on the crufty <ext/typelist.h> header
that uses macros to emulate variadic templates.
It relies on the <tr1/type_traits> header which is strictly inferior to
<type_traits>.
This pb_ds code itself is horribly obfuscated by macros. Many of the headers
follow the same structure:
#ifdef PB_DS_DATA_TRUE_INDICATOR
#define PB_DS_OV_TREE_NAME ov_tree_map
#define PB_DS_CONST_NODE_ITERATOR_NAME ov_tree_node_const_iterator_map
#endif
#ifdef PB_DS_DATA_FALSE_INDICATOR
#define PB_DS_OV_TREE_NAME ov_tree_set
#define PB_DS_CONST_NODE_ITERATOR_NAME ov_tree_node_const_iterator_set
#endif
#define PB_DS_CLASS_T_DEC \
template<typename Key, typename Mapped, typename Cmp_Fn, \
typename Node_And_It_Traits, typename _Alloc>
#define PB_DS_CLASS_C_DEC \
PB_DS_OV_TREE_NAME<Key, Mapped, Cmp_Fn, Node_And_It_Traits, _Alloc>
#define PB_DS_OV_TREE_TRAITS_BASE \
types_traits<Key, Mapped, _Alloc, false>
then the member function declarations use these:
PB_DS_CLASS_T_DEC
typename PB_DS_CLASS_C_DEC::const_iterator
PB_DS_CLASS_C_DEC::
find(key_const_reference r_key) const
As far as I can tell, none of these macros are documented or even commented to
explain what they are for or why they're used. When looking at any of the
ext/pb_ds/detail/foo/foo_imps.h headers they all look the same, defining
members of PB_DS_CLASS_C_DEC. It's a confusing mass of identical looking
declarations which doesn't give you any clue which class those members actually
belong to.
The PB_DS_DATA_TRUE_INDICATOR and PB_DS_DATA_FALSE_INDICATOR macros allow the
same header to be included twice, once to define ov_tree_map and once to define
ov_tree_set. However, there appears to be no good reason for the
PB_DS_CLASS_T_DEC macro, which just hides important details about the
template-head.
The PB_DS_OV_TREE_TRAITS_BASE macro is used exactly twice, and could just be
removed and types_traits<Key, Mapped, _Alloc, false> used directly in the two
places it's needed.
Similarly, PB_DS_DEBUG_MAP_BASE_C_DEC and PB_DS_TREE_TRACE_BASE_C_DEC are used
exactly twice each, and could just be removed and replaced with their
expansions.
These macros seem to be an attempt to make all the out-of-line member function
definitions look uniform, or to hide the fact they are templates so that nobody
has to read the C++ syntax. Whatever the reason, I hate it.
The PB_DS_CONST_NODE_ITERATOR_NAME macro is never used, neither are the
ov_tree_node_const_iterator_map and ov_tree_node_const_iterator_set names. Why
are they even here?
None of this fits in with the coding style or conventions elsewhere in the
library, increasing the maintenance cost.
