hsutter
[SUGGESTION][REFACTOR] Rework `is<T>(std::variant)` and `as<T>(std::variant)` to handle variadic number of types
Current implementation of is<T>(std::variant) and as<T>(std::variant) is complicated and limited only to 10 types hold in variant.
They can be implemented with funtions from standard libraries:
-
std::holds_alternative, and -
std::get
That will be less complicated and will work for variadic number of types.
I have recreated the same functionality - it should behave the same for the same inputs. Working prototype running on gcc, clang, and msvc: https://godbolt.org/z/hEcn58o56 (ìs2() and as2() are the new versions).
I was unable to recreate the state when valueless_by_exception() returns true. Analysing the code it should behave the same.
The new implementation forbids having duplicated types on the variant type list - types must be unique. In the current implementation the match is done to the first type on the list. In this change it ends with static assert failure
variant:1114:42: error: static assertion failed: T must occur exactly once in alternatives
1114 | static_assert(__detail::__variant::__exactly_once<_Tp, _Types...>,
Current implementation will behave like this:
std::variant<int, double, int> bad_one;
bad_one.emplace<2>(42);
std::cout << "Bad_one: (is: " << is<int>(bad_one) << "), (as: " << as<int>(bad_one) <<")" << std::endl;
the as<int>(bad_one) will cast bad_one to int using get<0>(bad_one) and will print:
Bad_one: (is: 1), (as: 42)
The question is if current behaviour is the one we shall expect?
I see that std::get_if is used in the code which also requires that types occurs exactly ones in alternatives: https://godbolt.org/z/aoPzdxqh1
Probably my implementation is consistent with the rest of the code.
Ensure that call to std::holds_alternative is well formed by checking if T appears exactly once on the type lists
Checks if count_t<T, Ts...> == 1 before calling std::holds_alternative:
// std::holds_alternative is ill-formed if T does not appear exactly once in Ts
if constexpr (count_t<T, Ts...> == 1) {
return std::holds_alternative<T>(x);
where
template <typename T, typename... Types>
inline constexpr std::ptrdiff_t count_t = ( std::ptrdiff_t{std::is_same_v<T, Types>} + ... );
Make is() and as() functions constexpr.
If the variant contains one type multiple times, the is() & as() methods will end up with static_assert failure.
Sorry for the lag on this:
The new implementation forbids having duplicated types on the variant type list - types must be unique.
I had been deliberately supporting a variant<int,int> case ... for example, right now this
main: () -> int = {
v: std::variant<int, int, std::string> = "xyzzy" as std::string;
test_generic(v);
v.emplace<0>(-);
test_generic(v);
v.emplace<1>(1);
test_generic(v);
}
test_generic: ( x: _ ) = {
std::cout
<< inspect x -> std::string {
is int = "int " + std::to_string(x as int);
is _ = "not an int";
}
<< "\n";
}
will print this
not an int
int 0
int 1
I think with this change this would no longer work, is that right?
The currently proposed change will make that code illegal. If you want to support that I can rework that to iterate over types on the list - that will be the same functionality but done without standard functions (they usually require that type T is not duplicated in the variant).
Just a side note. The code you have provided does not work due to the current implementation of CPP2_UFCS that fails with an error.
I've run into this error before: https://github.com/hsutter/cppfront/issues/82
Full error message:
% clang++ -std=c++20 ../tests/variant_tests.cpp -Iinclude/ && ./a.out
../tests/variant_tests.cpp2:5:5: error: expected ')'
CPP2_UFCS(emplace<0>, v, 0);
^
include/cpp2util.h:523:109: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^
../tests/variant_tests.cpp2:5:5: note: to match this '('
include/cpp2util.h:523:70: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^
../tests/variant_tests.cpp2:5:5: error: expression contains unexpanded parameter pack 'params'
CPP2_UFCS(emplace<0>, v, 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/cpp2util.h:523:19: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^ ~~~~~~ ~~~~~~
../tests/variant_tests.cpp2:5:5: error: expected ')'
include/cpp2util.h:524:96: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^
../tests/variant_tests.cpp2:5:5: note: to match this '('
include/cpp2util.h:524:57: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^
../tests/variant_tests.cpp2:5:5: error: expression contains unexpanded parameter pack 'params'
CPP2_UFCS(emplace<0>, v, 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/cpp2util.h:524:16: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^ ~~~~~~ ~~~~~~
error: constexpr if condition is not a constant expression
../tests/variant_tests.cpp2:5:5: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::variant<int, int, std::string> &, int>' requested here
include/cpp2util.h:528:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
^
../tests/variant_tests.cpp2:5:15: error: use of undeclared identifier 'emplace'
CPP2_UFCS(emplace<0>, v, 0);
^
../tests/variant_tests.cpp2:8:5: error: expected ')'
CPP2_UFCS(emplace<1>, v, 1);
^
include/cpp2util.h:523:109: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^
../tests/variant_tests.cpp2:8:5: note: to match this '('
include/cpp2util.h:523:70: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^
../tests/variant_tests.cpp2:8:5: error: expression contains unexpanded parameter pack 'params'
CPP2_UFCS(emplace<1>, v, 1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/cpp2util.h:523:19: note: expanded from macro 'CPP2_UFCS'
if constexpr (requires{ std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); }) { \
^ ~~~~~~ ~~~~~~
../tests/variant_tests.cpp2:8:5: error: expected ')'
include/cpp2util.h:524:96: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^
../tests/variant_tests.cpp2:8:5: note: to match this '('
include/cpp2util.h:524:57: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^
../tests/variant_tests.cpp2:8:5: error: expression contains unexpanded parameter pack 'params'
CPP2_UFCS(emplace<1>, v, 1);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/cpp2util.h:524:16: note: expanded from macro 'CPP2_UFCS'
return std::forward<decltype(obj)>(obj).FUNCNAME(std::forward<decltype(params)>(params)...); \
^ ~~~~~~ ~~~~~~
error: constexpr if condition is not a constant expression
../tests/variant_tests.cpp2:8:5: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::variant<int, int, std::string> &, int>' requested here
include/cpp2util.h:528:2: note: expanded from macro 'CPP2_UFCS'
}(PARAM1, __VA_ARGS__)
^
../tests/variant_tests.cpp2:8:15: error: use of undeclared identifier 'emplace'
CPP2_UFCS(emplace<1>, v, 1);
^
12 errors generated.
At least on my compiler:
% clang++ --version
Apple clang version 14.0.0 (clang-1400.0.29.202)
Target: arm64-apple-darwin22.1.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
@hsutter If you want to keep the current behavior please take a look at https://godbolt.org/z/vGnzEWeYv
The only difference is that it is not returning nonsuch instead of that it throws std::bad_variant_access
@hsutter, regarding the problem with the CPP2_UFCS macros, we can introduce another macro for templated methods/functions that we can use when cppfront identifies that the function/method has template arguments.
Prototype available here: https://godbolt.org/z/W4zP1Govr
Unfortunately, it does not solve https://github.com/hsutter/cppfront/issues/82
@filipsajdak Thanks! Here's what I've done so far (see this commit: ec32963f1144174e39d4274ab72801cbc238c666)
- For UFCS, I've added your suggested
_TEMPLATEforms for.templateemissions... this makes the examples likev.emplace<1>(1);compile also on GCC and Clang (which I didn't notice because MSVC allowed it and I must have only tried those examples on MSVC before), and it passes regression. - For
variantexamples, I've added a variety of duplicate-type examples to the regressions so they'll cover that. - For
variantinspection, let's leave things unrolled for now... two reasons: (a) having some maximum number is good enough for now, and (b) because I want to stay closer to the P2392 pattern matching design where I really want to show the indexed optimization (see 2.3.4) that for cases likevariantwe can eliminate having to traverse the alternatives list at run time, because pattern matching folks consider it essential that be demonstrated. - I also removed the
typeid(x).name()from the regressions, which addresses #166 (thanks for pointing that out).
Note about cleanup for variant and UFCS: I think is/as for variant, and possibly UFCS without chaining, is probably okay for now to make progress on other things. (I really want to start implementing template parameter lists and user-defined types once I've caught up with the issues/PRs.) However, I'll work my way through the other issues/PRs that mention variant and UFCS chaining to see if there's anything else that's easy to incorporate now.
Again, thanks!
@hsutter I am okay with leaving it/as for the variant unrolled.
I appreciate your work on cppfront, and I am doing my Best to support your goals. If my current approach of sending PRs to various topics distracts you from your plan, I am open to adjusting my way of working.
If you are working on something and would benefit from my testing, hardening, verifying, or developing something in a specific area, just let me know what I Should focus on. My goal is to support your goals.
@filipsajdak No worries, your issues and PRs have been super helpful, including that they've exposed (and helped fix, including on this thread) some significant limitations. Much appreciated! I was just trying to put in a note that I may defer some things till later, but right now I'm still working on incorporating as many as possible. Thanks!