P0896R4 changes to insert iterators

[CaseyCarter]

Description

When defined(__cpp_lib_concepts), (back_|front_|)insert_iterator are default constructible, and have ptrdiff_t as difference type.

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

• Identifiers in product code changes are properly _Ugly as per
  https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
• The STL builds successfully and all tests have passed (must be manually
  verified by an STL maintainer before automated testing is enabled on GitHub,
  leave this unchecked for initial submission).
• These changes introduce no known ABI breaks (adding members, renaming
  members, adding virtual functions, changing whether a type is an aggregate
  or trivially copyable, etc.).
• These changes were written from scratch using only this repository,
  the C++ Working Draft (including any cited standards), other WG21 papers
  (excluding reference implementations outside of proposed standard wording),
  and LWG issues as reference material. If they were derived from a project
  that's already listed in NOTICE.txt, that's fine, but please mention it.
  If they were derived from any other project (including Boost and libc++,
  which are not yet listed in NOTICE.txt), you must mention it here,
  so we can determine whether the license is compatible and what else needs
  to be done.

[miscco]

Loogs good even when I dread the merge conflicts

[miscco]

After thinking about it a bit more I have to take my approval back.

We just handed the users a great way to shoot themselfs in the foot and speaking from experience they will gladly do so. I would assume we should mitigate against some of that:

1. Any operation on a default constructed *_insert_iterator is an invalid operation. So I would suggest

ifdef __cpp_lib_concepts
#if _ITERATOR_DEBUG_LEVEL >= 1
        _STL_VERIFY(container, "cannot {dereference, increment} value-initialized  *_insert_iterator");
#endif // _ITERATOR_DEBUG_LEVEL >= 1
ifdef // __cpp_lib_concepts

2. It feels rather inconsitent that we require the -real- constructors to be explicit and happily let the user default construct them anyway. Adding explicit requires an LWG-issue right?

3. If we are already looking at concepts should they actually be constrained by something along those lines

ifdef __cpp_lib_concepts
    requires requires(_Container& __c) { __c.push_back(); }
ifdef // __cpp_lib_concepts

Another LWG issue maybe?

[CaseyCarter]

1. Any operation on a default constructed *_insert_iterator is an invalid operation. So I would suggest

ifdef __cpp_lib_concepts
#if _ITERATOR_DEBUG_LEVEL >= 1
        _STL_VERIFY(container, "cannot {dereference, increment} value-initialized  *_insert_iterator");
#endif // _ITERATOR_DEBUG_LEVEL >= 1
ifdef // __cpp_lib_concepts

Dereference and increment (since they have no effects) are well-defined for all three insert iterator types. ++++++++***++*++**x, for example, is a perfectly valid lvalue that denotes x. The only invalid operations are assignment from lvalue/rvalue Container::value_type. Those operations immediately dereference this->container which is known to be a nullptr for default-initialized insert iterators. We conventionally don't guard operations that will "safely" be terminated due to null pointer dereference unless it's as part of a larger set of consistent checks.

2. It feels rather inconsistent that we require the -real- constructors to be explicit and happily let the user default construct them anyway. Adding explicit requires an LWG-issue right?

WG21 traditionally annotates single-argument converting constructors as explicit to prevent inadvertent implicit conversions. There have been some rumblings recently about making multiple-argument constructors explicit, but generally speaking default/copy/move constructors should be implicit.

3. If we are already looking at concepts should they actually be constrained by something along those lines

ifdef __cpp_lib_concepts
    requires requires(_Container& __c) { __c.push_back(); }
ifdef // __cpp_lib_concepts

Another LWG issue maybe?

This probably needs a design paper. For Ranges, Eric and I decided that our goal was to conceptualize the algorithms and we deliberately put other kinds of concept constraints out of scope. We didn't want standardize ad hoc/poorly designed concepts (not that we didn't have some) to cover specific cases that might later interfere with "proper" conceptualization of containers and/or iostreams.

[miscco]

Plesse do not trouble you too much with merge conflicts with other PRs. It is totally fine to resolve those in the other PR.

I will hiss and wail though but that is fine

[barcharcraz]

out of curiosity: Why isn't this one constexpr (in the standard)

[CaseyCarter]

I have absolutely no idea. (To be honest, I don't understand why we ever put an explicit constexpr on a defaulted default constructor.) These default constructors came from P0896, which crossed with P1032 that added constexpr to all of the members of the insert iterators, and apparently the editors decided to put constexpr on some subset of the default constructors for reasons not apparent to me.

[barcharcraz]

Looks good to me