[Bug libstdc++/110138] New: Extra constructor called when using basic_string::operator+

[Bug libstdc++/110138] New: Extra constructor called when using basic_string::operator+

[wwwhhhyyy333 at gmail dot com]

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

            Bug ID: 110138
           Summary: Extra constructor called when using
                    basic_string::operator+
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: wwwhhhyyy333 at gmail dot com
  Target Milestone: ---

Created attachment 55268
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55268&action=edit
Simplified test

complied with -std=c++20 -O0

GCC 12.3/Clang 16 outputs:
Alloc: 3
Alloc: 6
Alloc: 9
Alloc: 12

GCC 13.1 outputs:
Alloc: 3
Alloc: 7
Alloc: 11
Alloc: 15

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[wwwhhhyyy333 at gmail dot com]

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

--- Comment #1 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
operator+ now calls std::__cxx11::basic_string<char, std::char_traits<char>,
myAlloc_<char> >::get_allocator, and it will call the constructor again after
gimplify

__attribute__((nodiscard))
struct allocator_type std::__cxx11::basic_string<char, std::char_traits<char>,
myAlloc_<char> >::get_allocator (
const struct basic_string * const this)
{
  try
    {
      _1 = std::__cxx11::basic_string<char, std::char_traits<char>,
myAlloc_<char> >::_M_get_allocator (this);
      myAlloc_<char>::myAlloc_ (<retval>, _1);
      return <retval>;
    }
  catch
    {
      <<<eh_must_not_throw (terminate)>>>
    }
  __builtin_unreachable trap ();
}

Possibly caused by r13-3814-gc93baa93df2d45

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Why do you care how many times an allocator is copied? They should be cheap
(essentially free) to copy.

A far more interesting test would look at how many bytes are allocated for
string concatenation:

#include <iostream>
#include <string>

int n_ = 0;
int b_ = 0;

void alloc_cnt() {
    std::cout<< "Allocators: " << n_ << ", bytes: " << b_ << '\n';
}

template <class T>
struct myAlloc_ : std:: allocator<T> {
  myAlloc_() { }
  myAlloc_(myAlloc_&& r) { ++n_; }
  myAlloc_(const myAlloc_& r) { ++n_; }
  myAlloc_& operator=(const myAlloc_& other) { return *this; }

  T* allocate(std::size_t n) {
    T* ptr = std::allocator<T>::allocate(n);
    b_ += n * sizeof(T);
    return ptr;
  }
 template <typename U>
 struct rebind { typedef myAlloc_<U> other; };
};
using mystring = std:: basic_string<char, std:: char_traits<char>,
myAlloc_<char>>;


int main()
{
  mystring f_ = "abc00000000000000000", b_ = "def", c_ = "xyz";
  alloc_cnt ();

  auto a = f_ + "mno" + b_;
  alloc_cnt ();

  auto c = c_ + f_ + b_;
  alloc_cnt ();

  auto d = c_ + (f_ + b_);
  alloc_cnt ();

  return 0;
}

With GCC 12:

Allocators: 3, bytes: 21
Allocators: 6, bytes: 83
Allocators: 9, bytes: 114
Allocators: 12, bytes: 176

With GCC 13:

Allocators: 3, bytes: 21
Allocators: 7, bytes: 52
Allocators: 11, bytes: 83
Allocators: 15, bytes: 114


The program uses less memory to achieve the same result. This was changed by
r13-3814-gc93baa93df2d45 and is actually important.

The number of copies of allocators is a meaningless figure.

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Hongyu Wang from comment #0)
> GCC 12.3/Clang 16 outputs:
> Alloc: 3
> Alloc: 6
> Alloc: 9
> Alloc: 12

"Clang 16" here actually means "Any version of Clang with libstdc++ headers
from GCC 12".

The figures for Clang's own libc++ are different:

Alloc: 0
Alloc: 4
Alloc: 8
Alloc: 12

But again, this is meaningless. Nobody cares how many times an allocator is
copied.

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[wwwhhhyyy333 at gmail dot com]

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

Hongyu Wang <wwwhhhyyy333 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
(In reply to Jonathan Wakely from comment #3)
> (In reply to Hongyu Wang from comment #0)
> > GCC 12.3/Clang 16 outputs:
> > Alloc: 3
> > Alloc: 6
> > Alloc: 9
> > Alloc: 12
> 
> "Clang 16" here actually means "Any version of Clang with libstdc++ headers
> from GCC 12".
> 
> The figures for Clang's own libc++ are different:
> 
> Alloc: 0
> Alloc: 4
> Alloc: 8
> Alloc: 12
> 
> But again, this is meaningless. Nobody cares how many times an allocator is
> copied.

The original test intends to verify P1165R1 implementation and it uses a global
counter on allocator constructor to see if it is correctly selected, and
current change makes it copied twice so the result is not expected.

But yes, I agree the allocator constructor for string should be cheap, and the
original test should not rely on how many times the constructor was called to
verify P1165R1 (I suppose checks if soccc was called instead).

Thanks for the explanation, I will close this as invalid.

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Just use a stateful allocator and check that the correct state is propagated.

See testsuite/21_strings/basic_string/allocator/char/operator_plus.cc which
does exactly that.

[Bug libstdc++/110138] Extra constructor called when using basic_string::operator+

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
What matters is the "value" of the allocator, not how many times it gets
copied.