-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
<tuple>
: Overhaul tuple_cat()
for simplicity and less special-casing
#2833
<tuple>
: Overhaul tuple_cat()
for simplicity and less special-casing
#2833
Conversation
Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
tuple_cat() now sends tuple_size_v to _Tuple_cat1. This captures everything we need to know about the tuple-like types, so we don't need _View_as_tuple anymore. Then, _Tuple_cat1 sends make_index_sequence<_Nx>... to _Tuple_cat2. These are the individual "K" sequences that need to be concatenated (and transformed to produce the "I" sequences). Now, _Tuple_cat2 can directly concatenate index_sequence<_Kx..., _Kx_next...>, so we don't need _Cat_sequences anymore. To repeat _Ix_next, we don't need a _Repeat_for helper, we can just write an expression `(_Ix_next + (0 * _Kx_next))...` that ignores the pack. Finally, we should rename _Tuple_cat2's primary parameter from _Tuples to _Sequences because that's what it takes now. Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
…before _Tuple_cat2. Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
The "arg" in `_Kx_arg_seq` and `_Ix_arg_seq` didn't serve a purpose. Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
…tainers. Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
I've had to push an additional commit to fix an internal compiler test that was evaluating Changelog for the latest commit:
Ultimately, this results in a solution that is almost as elegant, except that it has to pass the meta-tuple's type into the struct machinery. It makes the (weird) abstract class scenario work, by never theoretically constructing any I am intentionally not adding coverage for that scenario here, as the compiler suite provides coverage, and it appears to be highly sensitive to compiler bugs (the test was originally added due to a compiler regression, and it is still sensitive to |
…ing (microsoft#2833) Co-authored-by: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
Resolves #260. Inspired by @ArtemSarmini's #461, although this takes a different approach. See #461 (comment) for rationale: despite the issue's title, this PR is not generalizing
tuple_cat()
beyondpair
andarray
at this time (except forranges::subrange
), nor is it usingget
as a customization point.This PR can be viewed as a series of transformations (see individual commits, each of which build and pass tests), but it's simpler to explain as an overall before-and-after change:
_Cat_sequences
,_View_as_tuple
, and_Repeat_for
. We'll achieve the same results in different ways.tuple_cat()
:_Ret
typedef instead oftype
, for increased consistency.return
statement fits on a single line and is easier to read._Tuple_cat()
:forward_as_tuple()
, so it's always a modifiable rvalue. Thus we can take_Ty _Arg
and pass_STD move(_Arg)
, there's no need to forward it._Ret
with braces._Tuple_cat1
:struct
here.tuple<_Tuples&&...>
to_Tuple_cat2
; this is the type of the meta-tuple (whatforward_as_tuple()
returns). There it will be referred to as_Ty
, exactly matching_Ty _Arg
.make_index_sequence<SIZES>...
to_Tuple_cat2
, which will work (almost) purely with index sequences. These sequences (patterns like<0, 1, 2>
,<0, 1, 2, 3, 4>
,<0, 1>
, etc.) will be concatenated to form the_Kx
sequence. They are also transformed to form the_Ix
sequence.tuple_size_v<_Remove_cvref_t<_Tuples>>
. This is part of how we're replacing_View_as_tuple
, by viewing the arguments through the lens oftuple_size_v
. The_Tuples
are being perfectly forwarded, so we need to remove references (and we may as well remove cv-qualifiers, since the_Remove_cvref_t
helper is optimized, even thoughtuple_size_v
would ignore them). We don't need the full power of decay, as arrays and functions aren't interesting here._Kx
and_Ix
indices, combined with the meta-tuple
_Ty
, are sufficient to determine the return type. This significantly simplifies that machinery._Tuple_cat2
's primary template:_Ty
(used at the very end of recursion).class _Kx_arg, class _Ix_arg, size_t _Ix_next
have the same meanings as before._Sequences
instead of_Tuples
.static_assert
(non-tuple-like inputs would have been rejected at thetuple_size_v
layer)._Tuple_cat2
has a specialization to handle empty_Sequences
when recursion is done._Ret
from_Kx
,_Ix
, and_Ty
.tuple
is a modifiable rvalue, so we can directly saytuple_element_t<_Ix, _Ty>
.tuple
will be lvalue references or rvalue references (that's whatforward_as_tuple()
does), which we need to remove. They can also be cv-qualified (depending on the original_Tuples
), and we must remove that, ortuple_element_t
will "helpfully" propagate cv-qualifiers. (That is,is_same_v<tuple_element_t<0, const tuple<int>>, const int>
is true, but we don't want such propagation here.) However, we don't need the full power ofdecay_t
which we were using earlier.tuple_element_t
to determine_Ret
completes the replacement of_View_as_tuple
- as long astuple_size_v
reports the size of a tuple-like "container" andtuple_element_t
extracts the element type, this will work._Tuple_cat2
's recursion specialization:_Ret
computation until the end, so it doesn't have to mentiontuple
a bunch of times), and is directly doing the remaining work._Kx
indices, we can directly sayindex_sequence<_Kx..., _Kx_next...>
._Ix
indices, now that we have a pack of_Kx_next
values (instead of types), we can expand directly instead of using_Repeat_for
to ignore the types. All we need is an expression that results in_Ix_next
repeatedly, and "depends on"_Kx_next
to drive the expansion but ignores its values.(_Ix_next + 0 * _Kx_next)...
makes this clear, while avoiding any "conditional expression is constant" warnings.I believe that the result is significantly simpler to understand, and prepares
tuple_cat()
for possible future developments wheretuple_size_v
andtuple_element_t
handle more types.Dev11_0000000_tuple_cat
provides coverage of different-lengthtuple
s,pair
andarray
inputs, reference elements, and even had some coverage of cv-qualification and value categories for the tuple-like objects. However, because the return type logic is the biggest change in this PR, and needed_Remove_cvref_t
to handle such scenarios, I am adding extra validation (to ensure that the return type is not influenced by the cv-quals and value categories of the tuple-like objects, and purely concatenates the element types within).@CaseyCarter noted that this automatically works with
ranges::subrange
which supports the tuple protocol; I am adding test coverage for that.