[inplace_function] usage at shared library boundaries about sg14

[Quuxplusone]

Hi @kamrann,
As you already know, "standard C++" doesn't say anything about shared libraries, so the short answer is that anything involving shared libraries in general won't be "supported," especially by a header-only library like this one.

I'd be very sympathetic if you could work up a specific test case that reproduces the issue on TravisCI; then we could discuss whether that specific test case is worth supporting, and how to support it, and so on. (And we could run the test case on TravisCI to prove that we'd fixed it, forever.)

Off the top of my head, I think the only ways to fix the kind of issue you're probably seeing would be either

• Replace all uses of empty_vtable with nullptr (whose representation will be the same on both sides of the shared-library boundary), and add null-checks to places like operator() and operator=. (This is what std::function usually does.) I resist this idea because it is slower and less elegant than the current empty_vtable idea.

• Maybe, add some #if NON_HEADER_ONLY mode that the client programmer could set to make empty_vtable a non-inline variable, and then some macro to place its definition in a single object file of the client programmer's choice. But this seems quite difficult both to implement and to use.

[kamrann]

Thanks for the quick response @Quuxplusone.
Yep that makes sense, I guess 'supported' was the wrong word. Essentially, not having much experience with shared libraries, I'm open to being told that trying to use the class in this way is just a bad idea for more general reasons. But if that's not necessarily the case, then obviously it would be nice if the library was flexible enough to make it feasible in practice, without any guarantees.

I'd be happy to try to create a reproduction test case, though haven't used TravisCI before so any quick pointers would be helpful - would the idea be to fork the SG14 repo, then add to the tests? If so, I guess the nature of the issue would mean it would require some Cmake changes and not really drop nicely into the existing unit test setup. Happy to have a go, just want to check so I'm not starting down the wrong path.

[Quuxplusone]

would the idea be to fork the SG14 repo, then add to the tests?

Yes, the GitHub model is fork-based:

• Fork WG21-SG14/SG14 to kamrann/SG14
• git clone [email protected]:kamrann/SG14.git locally
• git checkout -b feature-branch in your local copy; work in that feature branch; git push origin feature-branch
• Eventually, submit a "cross-fork pull request" from kamrann/SG14/feature-branch into WG21-SG14/SG14/master

I guess the nature of the issue would mean it would require some Cmake changes and not really drop nicely into the existing unit test setup.

Probably correct. I don't know that much about the CMake stuff; IIRC it was @p-groarke who set that all up. At first, you might skip anything to do with CMake, write your new test as a simple shell script, and just modify the script: line in .travis.yml to run your new test directly. Hopefully someone could help you integrate it better once you get to the pull-request stage.

[kamrann]

Integration didn't require too heavy changes in the end. I've added the tests to this branch in my fork: https://github.com/kamrann/SG14/tree/inplace-shared-lib
Since at this point the change is just new failing tests I assume there is no point in a PR, but if opening one makes it easier for you to try things, just let me know.

I added a few simple tests of using inplace_function across a shared library boundary, but the very first one demonstrate the issue, triggering an assert on Windows. Travis CI seems to indicate that this isn't a problem on Linux as all tests pass there. I actually struggle to understand how it works - the more I think about it, the more it makes sense that separate images have their own version of an inline static variable.

Anyway as to your proposed solutions, yes the first one would be the obvious one, perhaps that could be made a compilation conditional? Something along these lines might be possible and minimize added ugliness:

#ifndef NO_INLINE_STATIC

[existing empty_vtable definition]
template <...>
constexpr auto empty_vtable_ptr = std::addressof(empty_vtable<...>);
template <class F, class... Args>
using safe_invoke = std::invoke<F, Args...>;

#else

template <...>
constexpr auto empty_vtable_ptr = nullptr;
template <class F, class... Args>
auto safe_invoke(F f, Args... args) { if (f) return f(args); else throw...; }

#endif

I haven't looked much at the vtable implementation though, I guess there are more considerations.

The second approach of letting the client define the variable is something I've been experimenting with a bit in some of my own libraries, and I agree it's awkward, especially so in this case with it being a template. Though it would have the advantage of minimal changes to existing code.

[WPMGPRoSToTeMa]

@kamrann some related reading on this. I think you could try to load it manually via dlopen using some special flags like RTLD_LOCAL and/or RTLD_DEEPBIND to reproduce the issue.

Also, this global variable may be handled by STB_GNU_UNIQUE, so you could also try compiling with -fno-gnu-unique.