Skip to content
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

Remove template parameter Archive from InputBindingsMap and OutputBindingsMap #521

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

uentity
Copy link
Contributor

@uentity uentity commented Sep 25, 2018

Archive isn't directly needed for any content in binding
structs and required only to instantiate different global static
maps for different archives.

This commit turns InputBindingsMap and OutputBindingsMap into
non-templated structs and modifies binding maps to include archive's
typeid, such that they describe relation: typeid(Archive) -> per-archive binding map.

That way only two global static objects will be created in runtime (for
input and output bindings respectively) in each library/executable. Also
types of these globals are independent from included archives and known
beforehand.

…utBindingsMap`

`Archive` isn't directly needed for any content in binding
structs and required only to instantiate different global static
maps for different archives.

This commit turns `InputBindingsMap` and `OutputBindingsMap` into
non-templated structs and modifies binding maps to include archive's
typeid, such that they describe relation: `typeid(Archive) ->
per-archive binding map`.

That way only two global static objects will be created in runtime (for
input and output bindings respectively) in each library/executable. Also
types of these globals are independent from included archives and known
beforehand.
@uentity
Copy link
Contributor Author

uentity commented Sep 25, 2018

Reason for this change is discussed in issue #508

@uentity
Copy link
Contributor Author

uentity commented Oct 5, 2018

I know that there's one non-related commit with suppressing warnings, but what do you think about the main idea?

It literally changes nothing in public API, but at the same time allows me to unify serialization code across multiple shared libraries.

@dtmoodie
Copy link

If this works as you say, it would be a great addition since I currently am doing some funky work arounds to make this work correctly across shared library boundaries.

@uentity
Copy link
Contributor Author

uentity commented Dec 17, 2018

@dtmoodie I successfully apply this patch and use it in my project exactly to "unite" serialization code from multiple shared libraries at runtime.
Works fine. Because patch is relatively simple and non-affecting. Try it.

@BotellaA
Copy link

BotellaA commented Jan 4, 2019

Great PR!! @AzothAmmo What is missing to accept the merge?

@arekfu
Copy link

arekfu commented Jan 11, 2024

Does this PR still have a chance of being merged? We are facing the same problem as @uentity (separate shared libraries) and we would love to either see this integrated. A workaround would also be great!

@AzothAmmo I am also willing to rebase the PR if necessary. Thanks!

Edit: turns out the rebase is trivial. The rebased code is here.

Edit 2: I have opened a new PR (#812).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants