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 #812

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arekfu
Copy link

@arekfu arekfu commented Jan 11, 2024

This PR is a rebase of #521. I am not the original author of the code in this PR (credit: @uentity), but I am willing to push it through integration.

…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.
@AzothAmmo
Copy link
Contributor

Thank you for the rebase - the only thing holding this up and holding up most PRs is that I haven't gotten around to fixing the various CI pipelines, which inexplicably broke a while back.

@AzothAmmo AzothAmmo added this to the v1.3.3 milestone Jan 17, 2024
@uentity
Copy link
Contributor

uentity commented Jan 22, 2024

@arekfu Thanks for reraising this! I'm glad that finally my improvement can make it into upstream code base.
I also did several other useful changes back in the days, but as I thought project is a bit stagnating, I stopped bothering with C++14 support and lost intention to upstream...

pingelit added a commit to pingelit/poly-scribe that referenced this pull request Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants