[inplace_function] Opt-in "safe" default constructed inplace_function about sg14

[liarokapisv]

I want to also comment on this issue because it is more far reaching than it initially appears.

Not having a customization point for the default constructed inplace_function handler makes it unusable for embedded work for multiple reasons. One example is banning of exceptions which does not permit throw syntax at all in header files.

Currently inplace_function uses the SG14_INPLACE_FUNCTION_THROW macro to customize it's behavior. This makes it completely unusable as a proper library. Let me elaborate:

Say library A's A/inplace_function.hpp file includes SG14/inplace_function, uses it's macro to customize it's behavior and wraps it in it's own namespace as a::inplace_function.

Say library B also does the same and defines it's own b::inplace_function but perhaps with
a different SG14_INPLACE_FUNCTION_THROW definition.

Then when library C tries to include both A and B, due to include guards / #pragma once B 's include of SG14/inplace_function won't occur so it won't be able to customize it as it needs to. Instead it's going to use library A 's customized version.

This is completely breaking behavior and forces libraries to literally copy paste code in order to get around this issue. The customization of inplace_function needs to be addressed as a proper library customization feature and not using macros.

A proposal is to add a policy to inplace_function in order to allow users to customize the behavior of the default constructor. This should come at no cost since it won't affect inplace_function any more than the Capacity template parameter does.

[Quuxplusone]

@liarokapisv: See "The space of design choices for std::function" (2019-03-27). I firmly believe that if we added "policy parameters" for every different thing a user might want, we would cause more harm than benefit. The goal of library design should be design — we should figure out what most users want, and then implement it. If we just dump a bunch of template Tinkertoys on the floor and say "build your own type with whatever policies you think you want today," we're abdicating the library-design role and also increasing the difficulty of teaching (and learning) the SG14 library, because users will have to learn all the things it might do instead of the one thing it does. (And people who want that much control over the code are perfectly capable of cutting-pasting-and-editing this code to do exactly what they want, anyway.)

Right now, if you call a default-constructed inplace_function, you don't violate any contract; it will throw bad_function_call (or, if exceptions are turned off, then it does whatever you've defined SG14_INPLACE_FUNCTION_THROW to do — the default is UB).

I think it's perfectly reasonable to argue that calling a default-constructed inplace_function should be a contract-violation, and thus be UB all the time regardless of SG14_INPLACE_FUNCTION_THROW. This would eliminate the library's reliance on exception handling. This would make it slightly less of a drop-in replacement for std::function, though, which I think is why we haven't made that change yet.

Do you have any particular use-case for calling operator() on a default-constructed or moved-from inplace_function? Would it cause you any hardship if the library disallowed calling operator() on a default-constructed or moved-from inplace_function?

Previously: #113, #152.

[phillipjohnston]

I just want to say that I use this library on microcontroller-based embedded projects without exceptions... But I don't have a complex project structure with multiple inplace_function.hpp files embedded within multiple libraries. I have a single copy of the header within a framework.

Would it cause you any hardship if the library disallowed calling operator() on a default-constructed or moved-from inplace_function?

I'm +1 for this.

[liarokapisv]

One interesting use of the default constructor is callbacks with void return type. In those cases one may want to just ignore everything by default which reduces boilerplate on creation. One may want this to be undefined in which case they can terminate, throw or even call a custom error handler.

I am admitedly more interested in allowing usage of inplace_function in embedded projects than customizing the default constructor per se. However I have the following argument about not relying on the usage of SG14_INPLACE_FUNCTION_THROW macro to support this.

If the purpose of SG13_INPLACE_FUNCTION_THROW is for arbitrary customization then that's a poor choice due to the scenario demonstrated above. I am currently in a position that I use inplace_function in my codebase and I also want to use an external library that also uses inplace_function internally. As it is, one library's behaviour overrides the behaviour of the other library forcing both me and the author of the library to copy paste the header and also change the namespaces to avoid ODR violations.

If the purpose of SG14_INPLACE_FUNCTION_THROW is to just provide support for embedded projects then a simple SG14_DISALLOW_EXCEPTIONS macro config check would be better suited to this task but then the library needs to support a reasonable default.

If we accept that both the embedded usecase of not allowing exceptions is real and that it's also worth allowing users to define the behaviour of the default constructor in that case (I argue that it is useful since there is no catch-all error reporting approach in that case as is the case of exceptions) , then that's accepting that this is a real customization point of the library in which case I argue that a policy would be better suited to this task due to the multiple problems of the macro approach.

Personally I don't mind banning the default constructor altogether in an embedded setting. What I have seen some libraries do is to check if exceptions are disabled and just declare but not define the relevant function leading to a linker error in case of misuse. It's then easy to build a wrapper on top of that that also allows for a default constructor.

[AndWass]

Personally I don't mind banning the default constructor altogether in an embedded setting. What I have seen some libraries do is to check if exceptions are disabled and just declare but not define the relevant function leading to a linker error in case of misuse. It's then easy to build a wrapper on top of that that also allows for a default constructor.

This would work for me as well!

[Quuxplusone]

Personally I don't mind banning the default constructor altogether in an embedded setting.

Banning the default constructor wouldn't solve the "narrow contract" issue: a moved-from object is in the same "disengaged" state as a default-constructed one.

stdext::inplace_function<void()> f; f();  // UB okay?
stdext::inplace_function<void()> g = [](){}; f = std::move(g); g();  // UB okay?

Also, if we banned the default constructor then inplace_function wouldn't model C++2a std::semiregular. Default-constructibility is overrated, but for something that's supposed to play well with the STL, I'd be hesitant in practice to make it not-default-constructible.

If g() above is allowed to be out-of-contract and have UB, then we don't need to eliminate the default constructor; we just say that f() above is equally out-of-contract and has UB. Note that calling a default-constructed inplace_function would have UB, but merely having one (creating one, copying one, etc) wouldn't have UB.

[liarokapisv]

A default constructed inplace_function object does not have to be disengaged.
A default constructor just enforces the intended default semantics of the library author.
One could very well implement inplace_function so as to just ignore everything in a default constructed state. As I said in my last post this is very useful in the case of event handlers.

In the disengaged state there are again multiple approaches that one may take.
Ub is a valid approach, but one may also want to throw an exception, early abort, or call a custom event handler perhaps in a debug build.

But the whole discussion does not really have to do with either of these.
The point is, should the library support embedded projects?
If not that's fine.
If it should, then it needs to do away with the macro approach because it's just completely broken.
In this case there are two approaches:

• Not allow embedded developers to customize the disengaged state in an arbitrary way. In this case a simple configuration define should suffice and the library must settle on some sane default behavior.
• Allow embedded developers to customize the disengaged state which means choosing between UB, aborting, custom handler etc. Since using macros for this is broken, another approach must be taken that has first-class supported by the library and most likely also applies to all users not only to embedded developers.

As such there are three choices that can be taken:

• Do not support embedded developers.
• Support embedded developers with a switch and some default catch-all behavior.
• Add a custom configuration facility (such as policies) as a first class citizen of the library.