-
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
Proposal: Variadics #2240
base: trunk
Are you sure you want to change the base?
Proposal: Variadics #2240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made another pass without diving into the type checking bits, since those seem like something that could be handled in a separate proposal once the other issues are handled.
Co-authored-by: josh11b <[email protected]>
Co-authored-by: josh11b <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I think this is ready for leads review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all but the appendix (largely trusting Josh's review there) and generally pretty happy across the board. Left a bunch of comments, but mostly minor wording / presentation improvements.
Co-authored-by: Chandler Carruth <[email protected]> Co-authored-by: josh11b <[email protected]>
Co-authored-by: Carbon Infra Bot <[email protected]>
Co-authored-by: josh11b <[email protected]>
proposals/p2240.md
Outdated
```cpp | ||
template <ConvertibleToString... Ts> | ||
std::string StrCat(const Ts&... params) { | ||
std::string result; | ||
result.reserve((params.Length() + ... + 0)); | ||
StrCatImpl(&result, params...); | ||
return result; | ||
if constexpr (sizeof...(params) == 0) { | ||
return ""; | ||
} else { | ||
std::string result; | ||
result.reserve((params.Length() + ... + 0)); | ||
StrCatImpl(&result, params...); | ||
return result; | ||
} | ||
} | ||
|
||
void StrCatImpl(std::string* out) { return; } | ||
|
||
template <ConvertibleToString T, ConvertibleToString... Ts> | ||
void StrCatImpl(std::string* out, const T& first, const Ts&... rest) { | ||
out->append(first.ToString()); | ||
StrCatImpl(out, rest...); | ||
if constexpr (sizeof...(rest) > 0) { | ||
StrCatImpl(out, rest...); | ||
} | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josh11b asked me whether I'd write / prefer the form with if constexpr
(with two termination conditions) or the form with the extra function overload. I said I'd actually write this:
```cpp | |
template <ConvertibleToString... Ts> | |
std::string StrCat(const Ts&... params) { | |
std::string result; | |
result.reserve((params.Length() + ... + 0)); | |
StrCatImpl(&result, params...); | |
return result; | |
if constexpr (sizeof...(params) == 0) { | |
return ""; | |
} else { | |
std::string result; | |
result.reserve((params.Length() + ... + 0)); | |
StrCatImpl(&result, params...); | |
return result; | |
} | |
} | |
void StrCatImpl(std::string* out) { return; } | |
template <ConvertibleToString T, ConvertibleToString... Ts> | |
void StrCatImpl(std::string* out, const T& first, const Ts&... rest) { | |
out->append(first.ToString()); | |
StrCatImpl(out, rest...); | |
if constexpr (sizeof...(rest) > 0) { | |
StrCatImpl(out, rest...); | |
} | |
} | |
``` | |
```cpp | |
template <ConvertibleToString... Ts> | |
std::string StrCat(const Ts&... params) { | |
std::string result; | |
result.reserve((params.Length() + ... + 0)); | |
(out->append(params.ToString()), ...); | |
return result; | |
} | |
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too, but I don't think that's representative of what a typical C++ programmer would do in this situation, so this gets back to the question of whether we want the C++ examples to reflect typical usage or expert usage (see also this comment thread).
Co-authored-by: Richard Smith <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this LG, but should confirm that other leads don't want a last look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this LG, but should confirm that other leads don't want a last look.
Proposes a set of core features for declaring and implementing generic variadic
functions.
A "pack expansion" is a syntactic unit beginning with
...
, which is a kind ofcompile-time loop over sequences called "packs". Packs are initialized and
referred to using "pack bindings", which are marked with the
each
keyword atthe point of declaration and the point of use.
The syntax and behavior of a pack expansion depends on its context, and in some
cases by a keyword following the
...
:...
iteratively evaluates its operand expression, and treats the values as
successive elements of the tuple.
...and
and...or
iteratively evaluate a boolean expression, combiningthe values using
and
andor
, and ending the loop early if the underlyingoperator short-circuits.
...
iteratively executes a statement....
iteratively matches the elements of the scrutinee tuple. In conjunction with
pack bindings, this enables functions to take an arbitrary number of
arguments.