On Thu, 6 Oct 2022 at 12:47, Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
wrote:

> Jonathan Wakely wrote:
> > Changing the condition for using the buffer affects ABI.
>
> Only in the very narrow sense that having two slightly different versions
> of `function` floating around the source code technically violates the ODR.
> This was just a change in the condition under which *the constructor puts*
> a T into the buffer; once the T is in the buffer (or allocated on the
> heap), everything is still done through the same set of type-erased
> operations as before. A program might end up with one `function` holding a
> `Widget` in its buffer, and another `function` holding a `Widget` on the
> heap, but since the fact that they're even `Widget`s at all has been
> type-erased, that's actually totally fine.
>

I'm not familiar with the libc++ implementation, but for the libstdc++ one
it wouldn't work. The internals of std::function are a class template
instantiated with the type of the stored callable, and so whether it's
stored locally or not is a property of the specialization. If you change
the definition of the specialization, it's a real "this mangled name has
different behaviour now" ABI-breaking ODR violation.

It looks like libc++ decides whether the object is stored locally by
comparing a pointer to its own buffer, so once that's set in the ctor, any
code looking at the object will know where the callable is stored. I wonder
if I can safely migrate libstdc++ to something like that.



> *But*, the question is also moot, because I decided to do what you said
> below anyway (keep the SBO condition the same, relax the noexceptness of
> the constructor). :)
>
> > You can just make the noexcept-specifier false for this case instead. It
> would use the buffer,
> > but doing so might throw. So constructing the std::function can throw.
>
> I did this now. The down side, of course, is that this makes the questions
> "Does this Widget fit in the small buffer?" and
> "is_nothrow_convertible_v<Widget, function>?" give different answers,
>

Yes but only for types with non-throwing copies and throwing moves ... are
those types realistic?



> which defeats Patrice's purpose. *Although this is a minor issue
> compared to the gaping hole that is (#3) below.*
>
> My libc++ change also somehow re-exposed this old issue with
> libc++'s std::bind, which I have not yet had the time to investigate:
> https://github.com/llvm/llvm-project/issues/16759
>
> –Arthur
>
> On Mon, Oct 3, 2022 at 7:05 PM Jonathan Wakely <cxx_at_[hidden]> wrote:
>
>>
>>
>> On Mon, 3 Oct 2022 at 23:15, Arthur O'Dwyer <arthur.j.odwyer_at_[hidden]>
>> wrote:
>>
>>> On Mon, Oct 3, 2022 at 5:23 PM Patrice Roy via SG14 <
>>> sg14_at_[hidden]> wrote:
>>>
>>>> What about making the constructor conditionally noexcept? The
>>>>> implementation knows whether a given type will be stored on the heap, and
>>>>> if that type can be constructed without throwing, so can easily expose that.
>>>>>
>>>>> What people care about is a property of the constructor, so why not
>>>>> make it part of the constructor signature?
>>>>>
>>>>> You can static assert is_nothrow_constructible.
>>>>>
>>>>
>>>> If the SG14 contributors like this, we could turn it into a paper. It
>>>> would (IMO) have a good chance of being accepted. This would let people use
>>>> std::function without any risk of allocating in the cases where they care
>>>> about this, and at essentially no cost. And if someone thinks all
>>>> std::function cases in their codebase should be protected that way, they
>>>> could envision writing a make_function() factory that would do this at no
>>>> cost either, being essentially
>>>>
>>>> template <class F> std::function<F> make_function(F && f) {
>>>> static_assert(std::is_nothrow_constructible_v<std::function<F>>);
>>>> return { std::forward<F>(f) };
>>>> }
>>>>
>>>
>>> (1) You mean this:
>>>
>>> template<class Sig, class T>
>>> std::function<Sig> make_function(T&& t) noexcept {
>>> static_assert(noexcept( std::function<Sig>(std::forward<T>(t))
>>> ));
>>> return std::function<Sig>(std::forward<T>(t));
>>> }
>>>
>>> (2) Notice that "The ctor doesn't throw" implies "The ctor doesn't
>>> allocate," but not vice versa. For example
>>> struct Empty {
>>> Empty() {}
>>> Empty(const Empty&) {}
>>> Empty& operator=(const Empty&) { return *this; }
>>> };
>>> auto lam = [e = Empty()](){};
>>> auto f = make_function<void()>(lam); // static-assert fails because
>>> copying `lam` is potentially throwing, regardless of whether it fits in the
>>> small buffer
>>> Therefore there are annoying false positives.
>>>
>>> (3) Not all std::function creation can be funneled through
>>> `make_function`. Consider
>>> void call_it(std::function<void()> g);
>>> auto f = make_function<int()>([x=42]() { return x; }); // OK
>>> call_it(f); // not OK; the converting constructor from
>>> function<int()> to function<void()> will allocate
>>> Therefore there are dangerous false negatives.
>>>
>>> (4) In implementing this idea for libc++ just now, I rediscovered
>>> something we already know: adding noexcept-specifications is really really
>>> error-prone. Libc++ will use the small buffer for any type T that is small
>>> enough and nothrow-copy-constructible. However, in the process of
>>> constructing the innards into the small buffer, libc++ happens to invoke
>>> T's *move constructor*. If T's copy constructor is non-throwing, but
>>> T's move constructor throws, then the exception will slam into
>>> function::function(T)'s `noexcept` firewall and terminate the program. This
>>> is non-conforming. I fixed this by saying, "Okay, libc++ no longer stores T
>>> in the small buffer unless T is nothrow-copy-constructible *and*
>>> nothrow-move-constructible"... but that's no basis for a system of
>>> government.
>>>
>>
>> Changing the condition for using the buffer affects ABI. You can just
>> make the noexcept-specifier false for this case instead. It would use the
>> buffer, but doing so might throw. So constructing the std::function can
>> throw.
>>
>> Yes, that means sometimes is_nothrow_constructible is false even though
>> it doesn't use the buffer ... for types that have non-throwing copies but
>> throwing moves ... i.e. almost no types in the real world.
>>
>>
>>
>>>
>>> I think it would be reasonable for a vendor (such as libstdc++) to add
>>> conditional-noexcept to std::function's converting constructor. But it
>>> isn't sufficient to get you what you want, and it is very likely to
>>> introduce subtle corner-case bugs into the vendor's implementation IMHO.
>>> Therefore I *don't* think it would be good to *force* vendors to do it.
>>>
>>> I intend to get the libc++ patch working and make it available on my fork
>>> https://p1144.godbolt.org/z/hja8sdT6b
>>> hopefully by the end of the week (yet not within the next couple of
>>> hours). Then people could (theoretically) play around with it and see
>>> whether it suffices for them. I very much bet that it won't.
>>>
>>> –Arthur
>>>
>>