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

Headers under metal/ are not installed #122

Open
cpreh opened this issue Dec 1, 2019 · 3 comments
Open

Headers under metal/ are not installed #122

cpreh opened this issue Dec 1, 2019 · 3 comments

Comments

@cpreh
Copy link

cpreh commented Dec 1, 2019

CMake's install target only installs metal.hpp and nothing else. The documentation uses the more specific headers everywhere (i.e. #include <metal/list/accumulate.hpp> to use metal::accumulate) which suggested to me that using these is also supported.

@brunocodutra
Copy link
Owner

The recommended way of consuming Metal is through the single header generated by cmake. One can still use specific headers if so desired, but the include tree cannot be automatically installed by cmake, since this would lead to ambiguities as the same code would be present on two separate files.

Docs refer to the specific headers as a way of linking back to the source code so the user can easily find the implementation, but I do see how this can be confusing as it suggests one should be including those headers.

I'm not sure how to solve this conundrum though, any suggestions @Freundlich?

@cpreh
Copy link
Author

cpreh commented Dec 5, 2019

So as it is right now, if I want to use an installed version of metal, I basically have to use the single header metal.hpp. The only way to allow both include styles would be to install both: the single header file and the include tree, which would lead to code duplication as you mentioned. I'm not sure it's worth it. What we should definitely do, however, is to clarify this in the documentation. Maybe it might be an idea to make it explicit that you should include metal.hpp for everything. e.g. write something like metal::accumulate: #include <metal.hpp>, implemented in <metal/list/accumulate.hpp>. This way you can still find the implementation in the source code, but you shouldn't accidentally include the wrong header.

@brunocodutra
Copy link
Owner

What we should definitely do, however, is to clarify this in the documentation.

Absolutely, thanks for reporting.

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

No branches or pull requests

2 participants