Issue 2385: function::assign allocator argument doesn't make sense

2385. `function::assign` allocator argument doesn't make sense

Section: 22.10.17.3 [func.wrap.func] Status: C++17 Submitter: Pablo Halpern Opened: 2014-05-23 Last modified: 2017-07-30

Priority: 2

View all other issues in [func.wrap.func].

View all issues with C++17 status.

Discussion:

The definition of function::assign in N3936 is:

template<class F, class A>
  void assign(F&& f, const A& a);
Effects: function(allocator_arg, a, std::forward<F>(f)).swap(*this)

This definition is flawed in several respects:

The interface implies that the intent is to replace the allocator in *this with the specified allocator, a. Such functionality is unique in the standard and is problematic when creating, e.g. a container of function objects, all using the same allocator.
The current definition of swap() makes it unclear whether the objects being swapped can have different allocators. The general practice is that allocators must be equal in order to support swap, and this practice is reinforced by the proposed library TS. Thus, the definition of assign would have undefined behavior unless the allocator matched the allocator already within function.
The general rule for members of function is to supply the allocator before the functor, using the allocator_arg prefix. Supplying the allocator as a second argument, without the allocator_arg prefix is error prone and confusing.

I believe that this ill-conceived interface was introduced in the effort to add allocators to parts of the standard where it had been missing, when we were unpracticed in the right way to accomplish that. Allocators were added to function at a time when the allocator model was in flux and it was the first class in the standard after shared_ptr to use type-erased allocators, so it is not surprising to see some errors in specification here. (shared_ptr is a special case because of its shared semantics, and so is not a good model.)

The main question is whether this member should be specified with better precision or whether it should be deprecated/removed. The only way I can see to give a "reasonable" meaning to the existing interface is to describe it in terms of destroying and re-constructing *this:

function temp(allocator_arg, a, std::forward<F>(f));
this->~function();
::new(this) function(std::move(temp));

(The temp variable is needed for exception safety). The ugliness of this specification underscores the ugliness of the concept. What is the purpose of this member other than to reconstruct the object from scratch, a facility that library classes do not generally provide? Programmers are always free to destroy and re-construct objects — there is no reason why function should make that especially easy.

I propose, therefore, that we make no attempt at giving the current interface a meaningful definition of questionable utility, but simply get rid of it all together. This leaves us with only two questions:

Should we deprecate the interface or just remove it?
Should we replace it with an assign(f) member that doesn't take an allocator?

Of these four combinations of binary answers to the above questions, I think the ones that make the most sense are (remove, no) and (deprecate, yes). The proposed new interface provides nothing that operator= does not already provide. However, if the old (deprecated) interface remains, then having the new interface will guide the programmer away from it.

The proposed wording below assumes deprecation. If we choose removal, then there is no wording needed; simply remove the offending declaration and definition.

Previous resolution [SUPERSEDED]:

This wording is relative to N3936.
Change class template function synopsis, 22.10.17.3 [func.wrap.func], as indicated:
template<class R, class... ArgTypes>
class function<R(ArgTypes...)> {
  […]
  // 20.9.11.2.2, function modifiers:
  void swap(function&) noexcept;
  template<class F, class A> void assign(F&&, const A&);
  […]
};
Change 22.10.17.3.3 [func.wrap.func.mod] as indicated:
template<class F, class A> void assign(F&& f, const A& a);
Effects: *this = forward<F>(f);function(allocator_arg, a, std::forward<F>(f)).swap(*this)
To deprecation section [depr.function.objects], add the following new sub-clause:
Old assign member of polymorphic function wrappers [depr.function.objects.assign]
namespace std{
  template<class R, class... ArgTypes>
  class function<R(ArgTypes...)> {
    // remainder unchanged
    template<class F, class A> void assign(F&& f, const A& a);
    […]
  };
}
The two-argument form of assign is defined as follows:
template<class F, class A> void assign(F&& f, const A& a);
Requires: a shall be equivalent to the allocator used to construct *this.
Effects: this->assign(forward<F>(f));

[2015-05, Lenexa]

STL: I would ask, does anybody oppose removing this outright?
Wakely: I don't even have this signature.
Hwrd: And I think this doesn't go far enough, even more should be removed. This is a step in the right direction.
PJP: I'm in favor of removal.
Wakely: We've already got TS1 that has a new function that does it right. We could wait for feedback on that. I think this issue should be taken now.
Marshall: Then the goal will be to move to ready.

Proposed resolution:

This wording is relative to N4431.

Change class template function synopsis, 22.10.17.3 [func.wrap.func], as indicated:

template<class R, class... ArgTypes>
class function<R(ArgTypes...)> {
  […]
  // 20.9.12.2.2, function modifiers:
  void swap(function&) noexcept;
  template<class F, class A> void assign(F&&, const A&);
  […]
};

Change 22.10.17.3.3 [func.wrap.func.mod] as indicated:
```
template<class F, class A> 
  void assign(F&& f, const A& a);
```
~~-2- Effects: function(allocator_arg, a, std::forward<F>(f)).swap(*this)~~

2385. function::assign allocator argument doesn't make sense

2385. `function::assign` allocator argument doesn't make sense