Date: Thu, 6 Oct 2022 07:47:30 -0400
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.
*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, 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
>>
>
> 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.
*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, 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
>>
>
Received on 2022-10-06 11:47:42