Title
Questionable precondition on Sequence containers a.assign(n, t)
Status
new
Section
[sequence.reqmts]
Submitter
Kazutoshi Satoda

Created on 2016-05-08.00:00:00 last changed 3 months ago

Messages

Date: 2020-04-25.18:12:26

Proposed resolution:

This wording is relative to N4861.

  1. In [sequence.reqmts], edit Table [tab:container.seq.req] (Sequence container requirements) as indicated:

    Table 77 — Sequence container requirements (in addition to container) [tab:container.seq.req]
    Expression Return type Assertion/note
    pre-/post-condition
    […]
    a.assign(n, t) void Preconditions: T is Cpp17CopyInsertable into X
    and Cpp17CopyAssignable. t is not a reference into a.
    Effects: Replaces elements in a with n copies of t.
    Invalidates all references, pointers and iterators referring to the elements of a.
    For vector and deque, also invalidates the past-the-end iterator.
Date: 2020-04-15.00:00:00

[ 2020-04-25, Daniel syncs current wording with recent working draft ]

Date: 2020-04-25.18:12:26

[ 2016-05 Issues Telecon ]

Howard believes this should be NAD, but we tabled the discussion.

Previous resolution [SUPERSEDED]:

This wording is relative to N4582.

  1. In [sequence.reqmts], edit Table 107 (Sequence container requirements) as indicated:

    Table 107 — Sequence container requirements (in addition to container)
    Expression Return type Assertion/note
    pre-/post-condition
    […]
    a.assign(n, t) void Requires: T shall be CopyInsertable into X and CopyAssignable.
    pre: t is not a reference into a.
    Replaces elements in a with n copies of t.
Date: 2016-05-09.17:37:37

Please look through the following modifications on a value v of type vector<T>:

assert(v.size() > 0);
v.push_back(v[0]);
v.insert(v.begin(), v[0]);
v.resize(v.size() * 2, v[0]);
v.assign(v.size() * 2, v[0]);

All of these use an element of itself which may be moved or destroyed by the modification.

From what I see so far, the first three are required to work. Please see library issue 526 for validity of them.

But only the last one is undefined because it violates a precondition of a sequence container operation. I think this is too subtle.

Should it be like that, really?

The precondition is in Table 107 "Sequence container requirements" at the next of [sequence.reqmts] p3.

In Tables 107 and 108, X denotes a sequence container class, a denotes a value of X containing elements of type T, […] n denotes a value of X::size_type, […] t denotes an lvalue or a const rvalue of X::value_type, […]

[…]

Table 107 — Sequence container requirements (in addition to container)
Expression Return type Assertion/note
pre-/post-condition
[…]
a.assign(n, t) void Requires: T shall be CopyInsertable into X and CopyAssignable.
pre: t is not a reference into a.
Replaces elements in a with n copies of t.

I looked into the following implementations:

  • libc++ relies on the precondition.

    It deallocates first on n > capacity() case, see here.

  • libstdc++ doesn't rely on the precondition.

    It creates temporary vector(n, t) and swap() on n > capacity() case, see here.

  • MSVC relies on the precondition.

    It unconditionally does clear() and then insert(begin(), n, t). I looked into my local "%PROGRAMFILES(X86)%/Microsoft Visual Studio 14.0/VC/include/vector".

One drawback of libstdc++ implementation, I could find so far, is possibly increased peek memory usage (both old and new buffer exist at the same time). But, because the same can happen on the most other modifications, it seems a reasonable trade-off to remove the precondition to fill the subtle gap. Users who really needs less memory usage can do clear() and insert() by themselves.

I also found that basic_string::assign(n, c) is safe on this point. At [string.assign] p17:

basic_string& assign(size_type n, charT c);

Effects: Equivalent to assign(basic_string(n, c)).

Returns: *this.

This can be seen as another gap.

Looking back on the history, I found that the definition of assign(n, t) was changed at C++14 for library issue 2209. There were more restricting definitions like this:

void assign(size_type n, const T& t);

Effects:

erase(begin(), end());
insert(begin(), n, t);

I think the precondition was probably set to accept this old definition and is not required inherently. And if the less memory usage was really intended, the standard is now underspecifying about that.

History
Date User Action Args
2020-04-25 18:12:26adminsetmessages: + msg11236
2016-05-22 15:38:38adminsetmessages: + msg8143
2016-05-08 21:06:05adminsetmessages: + msg8108
2016-05-08 00:00:00admincreate