[Bug libstdc++/14245] New: problem with user-defined allocators in std::basic_string

[Bug libstdc++/14245] New: problem with user-defined allocators in std::basic_string

[cbotev at yahoo dot com]

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

-- 
           Summary: problem with user-defined allocators in
                    std::basic_string
           Product: gcc
           Version: 3.3.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: cbotev at yahoo dot com
                CC: gcc-bugs at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-02-25 09:48 -------
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? ;)

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pcarlini at suse dot de
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-02-25 09:48:57
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[cbotev at yahoo dot com]

------- Additional Comments From cbotev at yahoo dot com  2004-02-25 14:01 -------
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

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-02-25 14:23 -------
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?

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-02-25 15:43 -------
... 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?

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-02-25 15:53 -------
> 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...



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-03-28 16:27 -------
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



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |3.4.1


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] [3.4 only] problem with user-defined allocators in std::basic_string

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-04-21 10:11 -------
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



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] [3.4 only] problem with user-defined allocators in std::basic_string

[pcarlini at suse dot de]

------- Additional Comments From pcarlini at suse dot de  2004-04-21 10:12 -------
Fixed for 3.4.1.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245

[Bug libstdc++/14245] [3.4 only] problem with user-defined allocators in std::basic_string

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-06-07 22:24 -------
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



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14245