[Bug libstdc++/116641] New: [12/13/14/15 Regression] std::string move assignment incorrectly depends on POCCA

[Bug libstdc++/116641] New: [12/13/14/15 Regression] std::string move assignment incorrectly depends on POCCA

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116641

            Bug ID: 116641
           Summary: [12/13/14/15 Regression] std::string move assignment
                    incorrectly depends on POCCA
           Product: gcc
           Version: 15.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
                CC: oliver.rosten at googlemail dot com
  Target Milestone: ---

Oliver Rosten noticed this bug:

#include <string>

template<typename T>
struct Alloc
{
  using value_type = T;
  using propagate_on_container_swap = std::false_type;
  using propagate_on_container_copy_assignment = std::true_type;
  using propagate_on_container_move_assignment = std::false_type;

  Alloc(int id) : id(id) { }

  template<typename U>
    Alloc(const Alloc<U>& a) : id(a.id) { }

  T* allocate(unsigned long n) { return std::allocator<T>().allocate(n); }
  void deallocate(T* p, unsigned long n) { std::allocator<T>().deallocate(p,
n); }

  Alloc& operator=(const Alloc&) { throw; }

  bool operator==(const Alloc& a) const { return id == a.id; }
  bool operator!=(const Alloc& a) const { return id != a.id; }

  int id;
};


void
test_move_assign()
{
  Alloc<char> a1(1), a2(2);
  std::basic_string<char, std::char_traits<char>, Alloc<char>> s1(a1), s2(a2);

  s1 = "sooooooooooooooooooooooooooooooo long";
  s2 = std::move(s1);
}

int main()
{
  test_move_assign();
}

There should be no allocator assignment here, but since r10-327-gdb33efde17932f
string::assign considers POCCA (propagate_on_container_copy_assignment) and so
when operator=(basic_string&&) calls assign to do a deep copy, it also ends up
propagating the allocator. That shouldn't happen, because POCMA is false here.

The fix should be as simple as changing operator= to call _M_assign instead of
assign.

[Bug libstdc++/116641] [12/13/14/15 Regression] std::string move assignment incorrectly depends on POCCA

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116641

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-09-07
   Target Milestone|---                         |12.5
      Known to work|                            |9.5.0
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
      Known to fail|                            |10.1.0, 14.2.0
     Ever confirmed|0                           |1
           See Also|                            |https://github.com/llvm/llv
                   |                            |m-project/issues/47783

[Bug libstdc++/116641] [12/13/14/15 Regression] std::string move assignment incorrectly depends on POCCA

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116641

--- Comment #1 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:c07cf418fdde0c192e370a8d76a991cc7215e9c4

commit r15-3575-gc07cf418fdde0c192e370a8d76a991cc7215e9c4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Sep 10 14:25:41 2024 +0100

    libstdc++: std::string move assignment should not use POCCA trait
[PR116641]

    The changes to implement LWG 2579 (r10-327-gdb33efde17932f) made
    std::string::assign use the propagate_on_container_copy_assignment
    (POCCA) trait, for consistency with operator=(const basic_string&).
    However, this also unintentionally affected operator=(basic_string&&)
    which calls assign(str) to make a deep copy when performing a move is
    not possible. The fix is for the move assignment operator to call
    _M_assign(str) instead of assign(str), as this just does the deep copy
    and doesn't check the POCCA trait first.

    The bug only affects the unlikely/useless combination of POCCA==true and
    POCMA==false, but we should fix it for correctness anyway. it should
    also make move assignment slightly cheaper to compile and execute,
    because we skip the extra code in assign(const basic_string&).

    libstdc++-v3/ChangeLog:

            PR libstdc++/116641
            * include/bits/basic_string.h (operator=(basic_string&&)): Call
            _M_assign instead of assign.
            * testsuite/21_strings/basic_string/allocator/116641.cc: New
            test.

[Bug libstdc++/116641] [12/13/14 Regression] std::string move assignment incorrectly depends on POCCA

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116641

--- Comment #2 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-14 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:d5d6d3ff43c5166ead1787c4334553be26cc84da

commit r14-10662-gd5d6d3ff43c5166ead1787c4334553be26cc84da
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Sep 10 14:25:41 2024 +0100

    libstdc++: std::string move assignment should not use POCCA trait
[PR116641]

    The changes to implement LWG 2579 (r10-327-gdb33efde17932f) made
    std::string::assign use the propagate_on_container_copy_assignment
    (POCCA) trait, for consistency with operator=(const basic_string&).
    However, this also unintentionally affected operator=(basic_string&&)
    which calls assign(str) to make a deep copy when performing a move is
    not possible. The fix is for the move assignment operator to call
    _M_assign(str) instead of assign(str), as this just does the deep copy
    and doesn't check the POCCA trait first.

    The bug only affects the unlikely/useless combination of POCCA==true and
    POCMA==false, but we should fix it for correctness anyway. it should
    also make move assignment slightly cheaper to compile and execute,
    because we skip the extra code in assign(const basic_string&).

    libstdc++-v3/ChangeLog:

            PR libstdc++/116641
            * include/bits/basic_string.h (operator=(basic_string&&)): Call
            _M_assign instead of assign.
            * testsuite/21_strings/basic_string/allocator/116641.cc: New
            test.

    (cherry picked from commit c07cf418fdde0c192e370a8d76a991cc7215e9c4)