2402. basic_string(const basic_string& str, size_type pos, size_type n = npos) shouldn't use Allocator()

Section: 24.3.2.2 [string.cons] Status: NAD Submitter: Stephan T. Lavavej Opened: 2014-06-14 Last modified: 2016-03-07

Priority: 3

View all other issues in [string.cons].

View all issues with NAD status.

Discussion:

24.3.2.2 [string.cons] p3 specifies:

basic_string(const basic_string& str, size_type pos, size_type n = npos, const Allocator& a = Allocator());

But this implies that basic_string(str, pos) and basic_string(str, pos, n) use Allocator() instead of getting an allocator from str.

24.3.2.1 [string.require] p3 says "The Allocator object used shall be obtained as described in 23.2.1." 26.2.1 [container.requirements.general] p8 says "Copy constructors for these container types obtain an allocator by calling allocator_traits<allocator_type>::select_on_container_copy_construction on the allocator belonging to the container being copied.", but this isn't exactly a copy constructor. Then it talks about move constructors (which this definitely isn't), and finally says that "All other constructors for these container types take a const allocator_type& argument. […] A copy of this allocator is used for any memory allocation performed".

[2015-05-06 Lenexa: move to Open]

STL: there an allocator right there in str, why default-construct one

STL: my fix, which may not be right, splits out functions with and without allocators

JW: there are other ways to propagate the allocator from str to the new object

PJP: hard to get motivated about this one

JW: I think this is not a copy operation, this is init'ing a string from a range of characters which happens to originate in a string. It makes it inconsistent with the similar ctor taking a const char pointer, and if we had a std::string_view we wouldn't even have this ctor, and it wouldn't be possible to propagate the allocator.

STL: but people with stateful allocators want it to propagate

JW: I think the people using stateful allocators will alter the default behaviour of select_on_container_copy_construction so that it doesn't propagate, but will return a default-constructed one (to ensure a stateful allocator referring to a stack buffer doesn't leak to a region where the stack buffer has gone). So for those people, your proposed change does nothing, it changes one default-constructed allocator to a call to select_on_container_copy_construction which returns a default-constructed allocator. For other people who have different stateful allocators they can still provide the right allocator (whatever that may be) by passing it in.

STL: OK, that's convincing.

PJP: I agree with Jonathan

JW: would like to run both our arguments by Pablo in case I'm totally misrepresenting the expected users of allocator-traits stuff

[2015-10, Kona Saturday afternoon]

Everyone thinks this seems right.

STL: It'd be really weird if you copy from a string with a stateful allocator and you'd just default-construct a new allocator.

EF: We definitely need this for polymorphic allocators.

TK: Whether you think this is kind of copy-constructor or a constructor from raw string data, the new form in the PR is more flexible. You can still get the default-constructed allocator if you want, but conversely, getting the select_on_container_copy is really hard to type in the old form.

JW has objections (written in the issue) but won't block "Review" status.

Move to Review, hopefully to be made ready at a telecon.

[2015-11-22, Pablo comments]

I like the direction of the PR, but it is incomplete. Consider the following (assuming the PR):

typedef basic_string<char, char_traits<char>, A<char>> stringA;
vector<stringA, scoped_allocator_adaptor<A<stringA>>> vs;
stringA s;

vs.emplace_back(s, 2);  // Ill-formed

The problem is that uses-allocator construction requires that we be able to pass an allocator to the constructor stringA(s, 2, allocator), but no such constructor exists. I think this defect already exists, but we should fix it a the same time that we fix 2402. So, I would say we need a third constructor:

basic_string(const basic_string& str, size_type pos, const Allocator& a);

[2016-01-05, Pablo comments]

I've reconsidered and I think that the issue as stated, is NAD. I do not like the PR at all. In fact, I think it reverses a previous fix and it could break existing code.

There are two patterns that are at work here:

  1. Every constructor needs a version with and without an allocator argument (possibly through the use of default arguments).

  2. Every constructor except the copy constructor for which an allocator is not provided uses a default-constructed allocator.

The constructors in question are not copy constructors. I do not think it is compelling that the allocator should come from its argument any more than it should come from any other object that happens to supply characters for a string constructor.

[2016-03 Jacksonville]

Closed as NAD, noting that 2583 is a related issue.

Previous resolution [SUPERSEDED]:

This wording is relative to N3936.

  1. Change 24.3.2 [basic.string] p5, class template basic_string synopsis, as indicated:

    […]
    // 21.4.2, construct/copy/destroy:
    […]
    basic_string(basic_string&& str) noexcept;
    basic_string(const basic_string& str, size_type pos, size_type n = npos);
    basic_string(const basic_string& str, size_type pos, size_type n = npos,
                const Allocator& a = Allocator());
    […]
    
  2. Change 24.3.2.2 [string.cons] around p3 as indicated:

    basic_string(const basic_string& str, 
                 size_type pos, size_type n = npos);
    basic_string(const basic_string& str, 
                 size_type pos, size_type n = npos,
                 const Allocator& a = Allocator());
    

    […]

    -5- Effects: Constructs an object of class basic_string and determines the effective length rlen of the initial string value as the smaller of n and str.size() - pos, as indicated in Table 65. The first constructor obtains an allocator by calling allocator_traits<allocator_type>::select_on_container_copy_construction on the allocator belonging to str.

    Table 65 — basic_string(const basic_string&, size_type, size_type) and basic_string(const basic_string&, size_type, size_type, const Allocator&) effects

Proposed resolution:

The existing wording is intended.