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

Add an implementation of std::variant #48

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

tcbrindle
Copy link
Owner

This PR adds Michael Park's variant implementation, and uses it in common_iterator (unless we're compiling with C++17, in which case std::variant is used).

This shrinks common_iterator a little, but at the cost of potentially worse codegen for various functions: in particular, operator==() gets routed through std::visit, which may not optimise all that well.

There's also the fact that MPark.Variant is absolutely huge -- at around 3kloc, it's about 20% the size of the whole of NanoRange at present. If it were just for common_iterator, I probably wouldn't bother; but we may be able to reuse it to implement semiregular, and thus avoid having to drag in an equally-large optional implementation from somewhere.

These functions act like std::get<T>(v), except that they do not throw if the current alternative is not T -- they (probably) just crash.
@CaseyCarter
Copy link
Contributor

If you're interested in something lighter-weight that is completely untested, I threw together a __variant_ish type to implement common_iterator in my partial libc++ port (https://github.com/CaseyCarter/libcxx/blob/libcmc%2B%2B/include/iterator#L2931-L3109).

std::variant is part of C++17, right? So if a compiler claims it supports C++17 via the __cplusplus macro, then <variant> should be available and usable, right?

Oh, you naive fool.

Firstly, std::variant is indeed available in GCC/libstdc++ 7, but unfortunately its move constructor isn't constexpr. This is presumably a bug that was fixed in version 8.

Furthermore, Apple flat-out lies about C++17 support. For AppleClang versions less then 10, the <variant> header simply doesn't exist; for version 10.0 (current at time of writing)  the header exists and the symbols are there, but they're all deleted unless you're also compiling on the latest MacOS (v10.14, even though XCode 10 is also availabke for MacOS 10.13). I have no idea whether it's possble to detect the MacOS version via a macro or not.

Finally, MSVC seems to be fine, presumably because we only actually support the latest version. Except in C++14 mode, where we use MPark.Variant, which gives apparently attempts to access the wrong union member in some constexpr code...
@tcbrindle
Copy link
Owner Author

Thanks Casey, I'll take a look. How many standard libraries are you implementing now?!

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.

None yet

2 participants