GCC/libstdc++ 3.3.3-0pre3 Debian Linux Testing("Sarge") It looks like the STL implementation of libstdc++ 3.3.3.0pre3 assumes that the user-defined allocators (at least for basic_string) should be initialized using the default constructor. In particular, the copy constructor for the string representation (struct _Rep) for basic_string uses the default constructor of the allocator(basic_string.tcc:192). I think this is wrong: the allocator may need additional info for proper initialization. It is also unnecessary. Simply using the copy constructor should be enough. I think the new line should look something like: : _M_dataplus(__str._M_rep()->_M_grab(_Alloc(__str.get_allocator()), __str.get_allocator()), Cheers, Chavdar
Hi. Honestly, I'm finding your report a little misleading: there is nothing wrong with using the default constructor: its availability is among the requirements for allocators (20.1.5, Table 32), and you find it everywhere in basic_string. Anyway... probably I get your point: you mean that, at string construction time, we should use _M_refcopy (not _M_clone) irrespective of the allocator used for __str, in other terms, _Alloc() is not special. Do you agree with my exegesis? ;)
No. I tend to disagree. My point is that the copy constructor of basic_string should provide "the most possible information" to the constructor of the allocator. In the case of the copy constructor of basic_string, this means invoking the copy constructor of the allocator and not the default constructor. I don't have the C++ standard handy: does it require a copy constructor for the allocators? Let me give you an example. The project I work on uses memory pools. We have allocators that provide an interface for the STL objects to a memory pool. To be properly initialized, these allocators need to get a pointer to a memory pool. Therefore, we cannot possibly initialize the allocator using the default constructor. On the other hand, if the copy constructor of the allocator is invoked, it can use the memory pool associated with the other allocator. I am also not sure what do you mean by using _M_refcopy instead of _M_clone. Do you mean changing _M_grab()? If yes, this is not correct. _M_grab() is also used by the assignment operator and the latter needs the _M_clone() functionality. If you mean using _M_refcopy() in the copy constructor of basic_string (instead of _M_grab()), I also think this may be incorrect. You don't know what the constructor of the allocator does (whether the default or the copy constructor). For example, in the case of memory pools, it may decide to create/use another memory pool. In this case, the string should be cloned and not copied. In any case, if the C++ standard requires the implementation of a copy constructor for allocators, then I think it should be used in the copy constructor of basic_string. Then, the implementation of the == operator for allocators will determine, whether _M_refcopy() or _M_clone() will be used in _M_grab(). If the copy constructor for allocators is not required, I guess I have to live with my current workaround: subclassing basic_string and implementing the copy constructor using the assignment operator instead of the copy constructor of basic_string. Presumably, it is less efficient but it does the job. Cheers, Chavdar
Ok. Now I see what you mean: 21.3.5: "... In the first form the Allocator value used is copied from str.get_allocator()" Have you got a testcase?
... of course I really meant 21.3.1, p5. Anyway, via the second argument passed to _M_dataplus we are *already* using the copy constructor to initialize the underlying _Alloc. What should be improved (notice, *improved* as a QoI issue, the current behavior is standard conforming) is what we do for the first argument: _M_grab (basing on _Alloc(), and __str.get_allocator()) either calls _M_refcopy or _M_clone(__alloc1), that is _M_clone(_Alloc()), that is does *not* use the allocator of the new string! In my opinion, we should first copy construct the allocator from __str, then either _M_refcopy or _M_clone (therefore allocate memory) via the allocator itself, the one of the string being constructed. Agreed?
> What should be improved (notice, *improved* as a QoI issue, the current > behavior is standard conforming) is what we do for the first argument: > _M_grab (basing on _Alloc(), and __str.get_allocator()) either calls > _M_refcopy or _M_clone(__alloc1), that is _M_clone(_Alloc()), that is does > *not* use the allocator of the new string! Actually *this* behavior seems wrong, not just a QoI issue. Bad things can happen at deallocation time, when the real allocator of the string will be used...
Subject: Bug 14245 CVSROOT: /cvs/gcc Module name: gcc Changes by: paolo@gcc.gnu.org 2004-03-28 16:27:27 Modified files: libstdc++-v3 : ChangeLog libstdc++-v3/include/bits: basic_string.tcc Log message: 2004-03-28 Chavdar Botev <cbotev@yahoo.com> PR libstdc++/14245 * include/bits/basic_string.tcc (basic_string::basic_string(const basic_string&)): Pass to _Rep::_M_grab the actual allocator of the string being constructed not the default constructed one. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2429&r2=1.2430 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&r1=1.67&r2=1.68
Subject: Bug 14245 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: paolo@gcc.gnu.org 2004-04-21 10:11:10 Modified files: libstdc++-v3 : ChangeLog libstdc++-v3/include/bits: basic_string.tcc Log message: 2004-04-21 Chavdar Botev <cbotev@yahoo.com> PR libstdc++/14245 * include/bits/basic_string.tcc (basic_string::basic_string(const basic_string&)): Pass to _Rep::_M_grab the actual allocator of the string being constructed not the default constructed one. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.78&r2=1.2224.2.79 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.46.4.4&r2=1.46.4.5
Fixed for 3.4.1.
Subject: Bug 14245 CVSROOT: /cvs/gcc Module name: gcc Branch: hammer-3_3-branch Changes by: paolo@gcc.gnu.org 2004-06-07 22:24:40 Modified files: libstdc++-v3 : ChangeLog.hammer libstdc++-v3/include/bits: basic_string.tcc Log message: 2004-06-08 Chavdar Botev <cbotev@yahoo.com> PR libstdc++/14245 * include/bits/basic_string.tcc (basic_string::basic_string(const basic_string&)): Pass to _Rep::_M_grab the actual allocator of the string being constructed not the default constructed one. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.hammer.diff?cvsroot=gcc&only_with_tag=hammer-3_3-branch&r1=1.1.2.16&r2=1.1.2.17 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/basic_string.tcc.diff?cvsroot=gcc&only_with_tag=hammer-3_3-branch&r1=1.28.2.7&r2=1.28.2.8