search

ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
704 stars 62 forks source link

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

Closed Lastique closed 5 years ago

Lastique commented 5 years ago

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 commented 5 years ago

revision.hpp is also affected.

ned14 commented 5 years ago

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 commented 5 years ago

I think so, yes.

ned14 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Fixed