-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[inplace_function] usage at shared library boundaries #170
Comments
Hi @kamrann, 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
|
Thanks for the quick response @Quuxplusone. 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. |
Yes, the GitHub model is fork-based:
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 |
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 I added a few simple tests of using 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:
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. |
@kamrann some related reading on this. I think you could try to load it manually via Also, this global variable may be handled by |
Seeing as
inplace_function
is standard layout and cannot allocate, I assumed it would be safe (dependent on what is put in it, of course) to be used across shared library interfaces. Apparently this is not the case.The mechanism used to identify an empty
inplace_function
relies on taking the address of aninline constexpr
global. Perhaps this is specific to MSVC (obviously this is not specified by the standard), but in practice I'm seeing a default constructed function converting to false in one module, but to true (callable) in another module due to differing addresses ofempty_vtable
.Is usage across shared libraries a bad idea in general and intentionally not supported? If not, is this something that can be easily addressed?
The text was updated successfully, but these errors were encountered: