Skip to content

fix: don't include <ciso646> if <version> is available#58

Merged
alandefreitas merged 4 commits intoalandefreitas:developfrom
tearfur:patch-2
Jan 10, 2026
Merged

fix: don't include <ciso646> if <version> is available#58
alandefreitas merged 4 commits intoalandefreitas:developfrom
tearfur:patch-2

Conversation

@tearfur
Copy link
Copy Markdown
Contributor

@tearfur tearfur commented Jan 9, 2026

Fixes #57.

@alandefreitas
Copy link
Copy Markdown
Owner

Hi, that's nice, thank you. The rationale is ciso646 gives us deprecated header warnings. But are we sure removing it doesn't break anything?

Unfortunately, CI is not being properly maintained for us to rely on it.

But cppreference says:

In old or nonconforming compilers, using the alternative operator representations may still require including this header.

I believe MSVC is in this set of compilers if I remember correctly. Or maybe was the other way around. As we see, defines the operators, but the other compilers don't do that by default. I'm not sure anymore. What I do know is that we shouldn't be using those operators anyway in library code, but that's a much bigger task.

@tearfur
Copy link
Copy Markdown
Contributor Author

tearfur commented Jan 10, 2026

My impression was <ciso646> was included for the feature testing macros that are being used later in cpp_version.cpp. AFAICT this library is not using the alternative operators. Edit: Oh it does use them.

Anyway, so what I'm understanding that someone has to fix the CI so that it goes green before this PR can be merged?


Separate question. How maintained is this library at the moment? From the past few interactions I had here, the hint I'm getting is something like "We welcome patches if you need something, but we are not planning to work on it ourselves".

CC @ckerr

@tearfur tearfur force-pushed the patch-2 branch 2 times, most recently from c70d0f8 to 51c52af Compare January 10, 2026 16:37
@tearfur tearfur force-pushed the patch-2 branch 2 times, most recently from d297c66 to 78f1c02 Compare January 10, 2026 16:58
@tearfur
Copy link
Copy Markdown
Contributor Author

tearfur commented Jan 10, 2026

@alandefreitas I removed all usages of the alternative operators, updated the existing CI pipelines to test the changes, and now all pipelines passed.

@alandefreitas
Copy link
Copy Markdown
Owner

My impression was was included for the feature testing macros that are being used later in cpp_version.cpp. > AFAICT this library is not using the alternative operators. Edit: Oh it does use them.

Yes, that's really unfortunate. We should never have used these.

Anyway, so what I'm understanding that someone has to fix the CI so that it goes green before this PR can be merged?

I noticed you fixed CI. Thank you so much for that! I actually wasn't going to block your PR because of that. There could be other solutions, but since you fixed it, that's a million times better. Thank you so much!

Separate question. How maintained is this library at the moment? From the past few interactions I had here, the hint I'm getting is something like "We welcome patches if you need something, but we are not planning to work on it ourselves".

Yes, that's pretty much the state the library is in. My day job is taking 100% of my time. And also, there are so many good competing libraries for this nowadays. New C++ features made small vectors are much easier to implement, C+26 actually includes an in-place vector, and there are so many good libraries for unordered containers like Boost.Unordered that is extremely performant nowadays.

@alandefreitas I removed all usages of the alternative operators, updated the existing CI pipelines to test the changes, and now all pipelines passed.

Yes, I had to look at that. That's amazing! Thank you so much for that!

@alandefreitas alandefreitas merged commit bab85b6 into alandefreitas:develop Jan 10, 2026
5 checks passed
@tearfur tearfur deleted the patch-2 branch January 11, 2026 04:30
@tearfur
Copy link
Copy Markdown
Contributor Author

tearfur commented Jan 11, 2026

Yes, that's pretty much the state the library is in.

Thanks for the transparency. Then it's probably a smart thing for us at the Transmission project to look for alternatives.

Having said that, I still think this is a great library, and I appreciate the fast reviews for my PRs.

@alandefreitas
Copy link
Copy Markdown
Owner

Transmission project to look for alternatives

The containers here are quite simple, and if I understand correctly, you guys are just using a small subset, right? Because in that case, you guys can just fork the project, or even just bundle the containers you like.

Having said that, I still think this is a great library, and I appreciate the fast reviews for my PRs.

Thank you, and thank you again for not only the contribution but for unblocking CI. And of course, if the library serves you guys as it it, PRs are always welcome, and I'll always do my best to review them.

Thank you!

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

Successfully merging this pull request may close these issues.

deprecated header warning for <ciso646> in detail/traits/cpp_version.hpp

2 participants