search

ned14 / status-code

Proposed SG14 status_code for the C++ standard
Other
64 stars 13 forks source link

`<cstring>` content gets potentially dragged into the `system_error2` namespace on APPLE #61

Closed BurningEnlightenment closed 1 year ago

BurningEnlightenment commented 1 year ago

https://github.com/ned14/status-code/blob/4f5f18c4f437ba5f17aac300875a12b2b8d661fc/include/status-code/posix_code.hpp#L36-L42

ned14 commented 1 year ago

Heh. That probably never was noticed due to extern "C". You want to fix it?

BurningEnlightenment commented 1 year ago

oh, the fix is even more trivial than I thought: https://github.com/ned14/status-code/blob/4f5f18c4f437ba5f17aac300875a12b2b8d661fc/include/status-code/posix_code.hpp#L34-L42 I will submit a PR right away.

ned14 commented 1 year ago

So you can't remove the <cstring> include on Apple, else stuff breaks. It was introduced only due to some other issue opened here. You've just undone that issue fix :)

I'd just move the include outside the system error 2 namespace.

BurningEnlightenment commented 1 year ago

But <cstring> is included unconditionally 5 lines prior?

BurningEnlightenment commented 1 year ago

@ned14 So now I'm mightily confused. Do we try to avoid including <cstring> in posix_code.hpp on Linux? Is the unconditional <cstring> include an oversight?

EDIT: For clarity, it currently looks like this. https://github.com/ned14/status-code/blob/0d814c64c7e3e8ce1e26178d0a47e1304198d10b/include/status-code/posix_code.hpp#L34-L40

ned14 commented 1 year ago

I missed that.

Including <cstring> the second time will be a noop because it's already been included.

Sorry about the noise.

BurningEnlightenment commented 1 year ago

Don't worry, I didn't notice it at first glance either 😄