Title
Contradictory requirements for string move assignment
Status
c++17
Section
[basic.string]
Submitter
Howard Hinnant

Created on 2011-05-29.00:00:00 last changed 89 months ago

Messages

Date: 2015-05-07.18:36:54

Proposed resolution:

This wording is relative to N4431.

  1. Modify the class template basic_string synopsis in [basic.string]:

    namespace std {
      template<class charT, class traits = char_traits<charT>,
        class Allocator = allocator<charT> >
      class basic_string {
      public:
        […]
        basic_string& assign(basic_string&& str) noexcept(
             allocator_traits<Allocator>::propagate_on_container_move_assignment::value ||
               allocator_traits<Allocator>::is_always_equal::value);
        […]
      };
    }
    
  2. Change [string.cons]/p21-23:

    basic_string&
    operator=(basic_string&& str) noexcept(
             allocator_traits<Allocator>::propagate_on_container_move_assignment::value ||
               allocator_traits<Allocator>::is_always_equal::value);
    

    -21- Effects: If *this and str are not the same object, modifies *this as shown in Table 71. [ Note: A valid implementation is swap(str). — end note ] Move assigns as a sequence container ([container.requirements]), except that iterators, pointers and references may be invalidated.

    -22- If *this and str are the same object, the member has no effect.

    -23- Returns: *this

    Table 71 — operator=(basic_string&&) effects
    Element Value
    data() points at the array whose first element was pointed at by str.data()
    size() previous value of str.size()
    capacity() a value at least as large as size()
  3. Modify the paragraphs prior to [string.assign] p.3 as indicated

    basic_string& assign(basic_string&& str) noexcept(
             allocator_traits<Allocator>::propagate_on_container_move_assignment::value ||
               allocator_traits<Allocator>::is_always_equal::value);
    

    -3- Effects: Equivalent to *this = std::move(str). The function replaces the string controlled by *this with a string of length str.size() whose elements are a copy of the string controlled by str. [ Note: A valid implementation is swap(str). — end note ]

    -4- Returns: *this

Date: 2015-05-15.00:00:00

[ 2015-05-07, Lenexa ]

Howard suggests improved wording

Move to Immediate

Date: 2012-08-11.00:00:00

[ 2012-08-11 Joe Gottman observes: ]

One of the effects of basic_string's move-assignment operator ([string.cons], Table 71) is

Element Value
data() points at the array whose first element was pointed at by str.data()

If a string implementation uses the small-string optimization and the input string str is small enough to make use of it, this effect is impossible to achieve. To use the small string optimization, a string has to be implemented using something like

union
{
   char buffer[SMALL_STRING_SIZE];
   char *pdata;
};

When the string is small enough to fit inside buffer, the data() member function returns static_cast<const char *>(buffer), and since buffer is an array variable, there is no way to implement move so that the moved-to string's buffer member variable is equal to this->buffer.

Resolution proposal:

Change Table 71 to read:

Element Value
data() points at the array whose first element was pointed at by str.data() that contains the same characters in the same order as str.data() contained before operator=() was called
Date: 2011-09-06.13:05:28

[ originally proposed wording: ]

This wording is relative to the FDIS.

  1. Modify the class template basic_string synopsis in [basic.string]:

    namespace std {
      template<class charT, class traits = char_traits<charT>,
        class Allocator = allocator<charT> >
      class basic_string {
      public:
        […]
        basic_string& operator=(basic_string&& str) noexcept;
        […]
        basic_string& assign(basic_string&& str) noexcept;
        […]
      };
    }
    
  2. Remove the definition of the basic_string move assignment operator from [string.cons] entirely, including Table 71 — operator=(const basic_string<charT, traits, Allocator>&&). This is consistent with how we define move assignment for the containers in Clause 23:

    basic_string<charT,traits,Allocator>&
    operator=(basic_string<charT,traits,Allocator>&& str) noexcept;
    

    -22- Effects: If *this and str are not the same object, modifies *this as shown in Table 71. [ Note: A valid implementation is swap(str). — end note ]

    -23- If *this and str are the same object, the member has no effect.

    -24- Returns: *this

    Table 71 — operator=(const basic_string<charT, traits, Allocator>&&)
    Element Value
    data() points at the array whose first element was pointed at by str.data()
    size() previous value of str.size()
    capacity() a value at least as large as size()
  3. Modify the paragraphs prior to [string.assign] p.3 as indicated (The first insertion recommends a separate paragraph number for the indicated paragraph):

    basic_string& assign(basic_string&& str) noexcept;
    

    -?- Effects: Equivalent to *this = std::move(str). The function replaces the string controlled by *this with a string of length str.size() whose elements are a copy of the string controlled by str. [ Note: A valid implementation is swap(str). — end note ]

    -3- Returns: *this

Date: 2015-05-07.18:36:54

[ 2011 Bloomington ]

Alisdair: Can this be conditional noexcept?

Pablo: We said we were not going to put in many conditional noexcepts. Problem is not allocator, but non-normative definition. It says swap is a valid operation which it is not.

Dave: Move assignment is not a critical method.

Alisdair: Was confusing assignment and construction.

Dave: Move construction is critical for efficiency.

Kyle: Is it possible to test for noexcept.

Alisdair: Yes, query the noexcept operator.

Alisdair: Agreed there is a problem that we cannot unconditionally mark these operations as noexcept.

Pablo: How come swap is not defined in alloc

Alisdair: It is in utility.

Pablo: Swap has a conditional noexcept. Is no throw move constructable, is no throw move assignable.

Pablo: Not critical for strings or containers.

Kyle: Why?

Pablo: They do not use the default swap.

Dave: Important for deduction in other types.

Alisdair: Would change the policy we adopted during FDIS mode.

Pablo: Keep it simple and get some vendor experience.

Alisdair: Is this wording correct? Concerned with bullet 2.

Pablo: Where does it reference containers section.

Alisdair: String is a container.

Alisdair: We should not remove redundancy piecemeal.

Pablo: I agree. This is a deviation from rest of string. Missing forward reference to containers section.

Pablo: To fix section 2. Only the note needs to be removed. The rest needs to be a forward reference to containers.

Alisdair: That is a new issue.

Pablo: Not really. Talking about adding one sentence, saying that basic string is a container.

Dave: That is not just a forward reference, it is a semantic change.

PJ: We intended to make it look like a container, but it did not satisfy all the requirements.

Pablo: Clause 1 is correct. Clause 2 is removing note and noexcept (do not remove the rest). Clause 3 is correct.

Alisdair: Not sure data() is correct (in clause 2).

Conclusion: Move to open, Alisdair and Pablo volunteered to provide wording

Date: 2011-05-29.00:00:00

[string.require]/p4 says that basic_string is an "allocator-aware" container and behaves as described in [container.requirements.general].

[container.requirements.general] describes move assignment in p7 and Table 99.

If allocator_traits<allocator_type>::propagate_on_container_move_assignment::value is false, and if the allocators stored in the lhs and rhs sides are not equal, then move assigning a string has the same semantics as copy assigning a string as far as resources are concerned (resources can not be transferred). And in this event, the lhs may have to acquire resources to gain sufficient capacity to store a copy of the rhs.

However [string.cons]/p22 says:

basic_string<charT,traits,Allocator>&
operator=(basic_string<charT,traits,Allocator>&& str) noexcept;

Effects: If *this and str are not the same object, modifies *this as shown in Table 71. [Note: A valid implementation is swap(str). — end note ]

These two specifications for basic_string::operator=(basic_string&&) are in conflict with each other. It is not possible to implement a basic_string which satisfies both requirements.

Additionally assign from an rvalue basic_string is defined as:

basic_string& assign(basic_string&& str) noexcept;

Effects: The function replaces the string controlled by *this with a string of length str.size() whose elements are a copy of the string controlled by str. [ Note: A valid implementation is swap(str). — end note ]

It seems contradictory that this member can be sensitive to propagate_on_container_swap instead of propagate_on_container_move_assignment. Indeed, there is a very subtle chance for undefined behavior here: If the implementation implements this in terms of swap, and if propagate_on_container_swap is false, and if the two allocators are unequal, the behavior is undefined, and will likely lead to memory corruption. That's a lot to go wrong under a member named "assign".

History
Date User Action Args
2017-07-30 20:15:43adminsetstatus: wp -> c++17
2015-05-22 18:31:21adminsetstatus: immediate -> wp
2015-05-07 21:02:13adminsetstatus: open -> immediate
2015-05-07 18:36:54adminsetmessages: + msg7366
2015-05-07 18:36:54adminsetmessages: + msg7365
2012-08-12 18:51:35adminsetmessages: + msg6123
2011-08-18 12:47:33adminsetstatus: new -> open
2011-08-16 12:46:19adminsetstatus: ready -> new
2011-08-16 10:45:53adminsetmessages: + msg5841
2011-08-16 10:45:53adminsetstatus: new -> ready
2011-05-31 19:48:27adminsetmessages: + msg5800
2011-05-29 00:00:00admincreate