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

version.hpp has no include guards and license header #184

Closed
Lastique opened this issue Apr 13, 2019 · 7 comments

Comments

@Lastique
Copy link

commented Apr 13, 2019

At least in Boost, all public headers (and boost/outcome/version.hpp is considered public, given its location) need to be self-sufficient and protected against double inclusion. Also, all headers must have a license comment at the top.

@Lastique

This comment has been minimized.

Copy link
Author

commented Apr 13, 2019

revision.hpp is also affected.

@ned14

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

Historically this was because the cmake scripting parses thoses files, hence them being small and simple and unconfusing to simple regex parsing. Is it critically important that this be fixed?

@Lastique

This comment has been minimized.

Copy link
Author

commented Apr 13, 2019

I think so, yes.

@ned14

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

I believe I can craft a licence which won't confuse the cmake parsers, so consider that done.

However the version and revision headers are deliberately and unintentionally guarded. I want them to warn when incompatible versions get mixed into the same translation unit.

Can I relocate them into detail, and you would be happy?

@Lastique

This comment has been minimized.

Copy link
Author

commented Apr 13, 2019

I believe I can craft a licence which won't confuse the cmake parsers, so consider that done.

Good, thanks.

I want them to warn when incompatible versions get mixed into the same translation unit.

The way they are now, they will warn about macro redefinitions regardless of whether the versions are the same or different.

Can I relocate them into detail, and you would be happy?

Sure, that would make them private, that'd be fine.

@ned14

This comment has been minimized.

Copy link
Owner

commented Apr 13, 2019

The way they are now, they will warn about macro redefinitions regardless of whether the versions are the same or different.

Compilers only warn if the macro definition changes. You can repeat a definition as many times as you like, so long as it is identical. This is specifically what I want to canary to end users.

I'll leave this issue open to remind me to make these changes, but be aware they will land no earlier than after the Cologne WG21 meeting, sorry. I have a hideous non-work workload between now and then. But all bug fixes to Outcome should land well in time for the August release.

@ned14 ned14 added the enhancement label Apr 15, 2019

@ned14 ned14 added this to the Boost 1.71 cutoff milestone Jun 17, 2019

ned14 added a commit that referenced this issue Jun 18, 2019
ned14 added a commit that referenced this issue Jun 18, 2019
ned14 added a commit that referenced this issue Jun 18, 2019
ned14 added a commit that referenced this issue Jun 18, 2019
@ned14

This comment has been minimized.

Copy link
Owner

commented Jun 18, 2019

Fixed

@ned14 ned14 closed this Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.