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

cannot include header in multiple place. #3

Closed
nukes opened this issue Nov 5, 2018 · 2 comments
Closed

cannot include header in multiple place. #3

nukes opened this issue Nov 5, 2018 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nukes
Copy link

nukes commented Nov 5, 2018

Describe the bug
As the implementation and declaration stays in one file, i can not include this file in different source files
if these files would be linked together. Because multiple definition exist in object files.

You may separate the implementation into .cc file ?

@nukes
Copy link
Author

nukes commented Nov 5, 2018

I see, you use inline for keep this header work in multiple files. But i find some methods are still not inline,which makes the problem.
I can successfully compile by fix the methods by add "inline" keyword, but i think it is not a good solution as the code size grows a lot in my case.
I think a separate the implementation into .cc file is still a good idea.

@gulrak
Copy link
Owner

gulrak commented Nov 5, 2018

Indeed some inlines where missing. I missed adding a test to ensure that including it into multiple implementation files works. I'll push a fix for that.

I understand, that there are arguments for a separated approach, but this is explicitly designed as a header-only/single-file solution, so currently there are no plans for separation into a .h/.cpp pair of files or even more files.

I once had an early version that supported separation by including the header with specific defines for implementation or forwarding header, but the effect was not as relevant as I had hoped, the code got quite a bit uglier to work with, and with link-time-optimization (-flto on gcc) most redundancies are dropped from the binary.
Still if multiple shared libraries are including this header, it might make more of a difference, but the results of that tests pushed the feature down the list.

gulrak pushed a commit that referenced this issue Nov 5, 2018
…when including in multiple files. Added a test.
@gulrak gulrak added the bug Something isn't working label Nov 5, 2018
@gulrak gulrak self-assigned this Nov 5, 2018
gulrak added a commit that referenced this issue Nov 5, 2018
…when including in multiple files. Added a test.
@gulrak gulrak closed this as completed Nov 5, 2018
@gulrak gulrak added this to the v1.0.4 milestone Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants