Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upTuple-Based Variadic Generics #1935
Conversation
cramertj
added some commits
Feb 23, 2017
aturon
added
the
T-lang
label
Feb 28, 2017
This comment has been minimized.
This comment has been minimized.
|
How would you actually write variadic functions under this proposal? Am I correct that you'd still have to write something like I guess the unresolved questions covers this? |
This comment has been minimized.
This comment has been minimized.
camlorn
commented
Feb 28, 2017
|
It is not a major deal breaker or anything, but it's worth noting that my field reordering stuff does currently affect tuples, and we probably want it to continue doing so where possible, I'd think. Now if only the world would leave me alone for the like 2 days I need to finish my field reordering stuff, but that's another discussion. |
This comment has been minimized.
This comment has been minimized.
|
@withoutboats You're correct, the current proposal would require something like |
eddyb
reviewed
Feb 28, 2017
| type AsRefs<'a>: Tuple + 'a; | ||
| type AsMuts<'a>: Tuple + 'a; | ||
| fn elements_as_refs<'a>(&'a self) -> Self::AsRefs<'a>; | ||
| fn elements_as_mut<'a>(&'a mut self) -> Self::AsMuts<'a>; |
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
I would prefer to avoid a hard dependency on ATC here, and I'm not sure the functionality needs to be in Tuple itself.
IOW, we can do this more transparently to the user with a recursive impl perhaps? Well, no, "compiler knows best", it could project AsRefs partially (e.g. (A, ...B, C)::AsRefs == (&A, ...B::AsRefs, &C)), but we can probably experiment with a recursive impl in a library, for a trait TupleAsRefs<'a>, to start with.
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
To be clear, it's likely we'll want to only stabilize the ATC form itself, for convenience's sake, but it's not needed to experiment IMO.
This comment has been minimized.
This comment has been minimized.
cramertj
Feb 28, 2017
Author
Member
Yeah-- I left a comment about this in the "unresolved questions" section. Separate traits (TupleAsRef<'a> and TupleAsMut<'b>) work and eliminate the dependency on ATCs, but I think it's slightly less clear to the end user.
eddyb
reviewed
Feb 28, 2017
|
|
||
| ## The `(Head, ...Tail)` Type Syntax | ||
| This syntax would allow for a `Cons`-cell-like representation of tuple types. | ||
| For example, `(A, ...(B, C))` would be equivalent to `(A, B, C)`. This allows |
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
This is potentially misleading, as (A, ...(B, C), D) would also be valid. Any mention of "cons" is likely to confuse more than help, compared to, say, "tuple concatenation" (which is valid for both types and values). See #1921 (comment) and #1921 (comment).
This comment has been minimized.
This comment has been minimized.
cramertj
Feb 28, 2017
Author
Member
I had actually purposefully avoided discussing things like (...(A, B,) C) (tuples in non-tail position). I wasn't sure if that was something we wanted to support in the initial implementation, but I think it makes sense to include them prior to stabilizing.
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
@sgrif thought too (#1921 (comment)) but I disagree, from the implementation side doing it right the first time is not much harder, and it's less work overall than having two incompatible implementations at different points in time.
This comment has been minimized.
This comment has been minimized.
cramertj
Feb 28, 2017
Author
Member
Good to know-- you have a much better sense of the actual implementation then I do. I'll edit the RFC appropriately.
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
•
Member
To be more clear: both tail-only and generalized single-repeat require the same primitive: a single check somewhere in the compiler that the T in ...T is now known to be a tuple (i.e. after substituting in type parameters, resolving associated types, etc.) therefore expand it.
And concatenating 3 lists of types is hardly any more difficult than concatenating 2 lists.
This is done only once, mind you (probably should be done when creating any tuple IMO).
On top of that, everywhere a tuple is introspected now has to take into account a partially unknown tuple, and that has the same cost for both approaches, except if you do it one way then later you have to redo in the other way, and there can even be subtle bugs introduced.
eddyb
reviewed
Feb 28, 2017
| let (head, ...tail) = self; | ||
| let mapped_head = f(head); | ||
| let mapped_tail = tail.map(f); | ||
| (mapped_head, ...mapped_tail) |
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
Might be a bit clearer with the last 3 lines flattened into (f(head), ...tail.map(f)).
eddyb
reviewed
Feb 28, 2017
| ``` | ||
|
|
||
| This example is derived from | ||
| [a playground example by @eddyb]() |
This comment has been minimized.
This comment has been minimized.
eddyb
reviewed
Feb 28, 2017
| -Should the `Tuple` trait use separate `TupleRef<'a>` and `TupleMut<'b>` traits | ||
| to avoid dependency on ATCs? It seems nicer to have them all together in one | ||
| trait, but it might not be worth the resulting feature-stacking mess. | ||
|
|
This comment has been minimized.
This comment has been minimized.
eddyb
Feb 28, 2017
Member
Overlap with range syntax is a problem. I believe x... could always work if we decide it doesn't make sense as a range, over x.. (i.e. is "inclusive at infinity" useful?).
This comment has been minimized.
This comment has been minimized.
scottmcm
Feb 28, 2017
Member
x... is how you expand a parameter pack in C++, so it'd be familiar to some, for better or worse.
eddyb
reviewed
Feb 28, 2017
| lists, the concept should be introduced using "structural recursion": there's | ||
| a base case, `()`, and a recursive/inductive case: `(Head, ...Tail)`. Any tuple | ||
| can be thought of in this way | ||
| (for example, `(A, B, C)` is equivalent to `(A, ...(B, ...(C, ...())))`). |
This comment has been minimized.
This comment has been minimized.
cramertj
added some commits
Feb 28, 2017
cramertj
force-pushed the
cramertj:variadic-generics
branch
from
dbe25b8
to
8114d4c
Feb 28, 2017
This comment has been minimized.
This comment has been minimized.
|
@camlorn Right, and nothing in the RFC assumes field order matters, i.e. there are no "partial tail borrows" or anything like that, it can all be implemented in a library with safe code for a fixed number of arities. |
This comment has been minimized.
This comment has been minimized.
fn foo<H, T: Trait>((head, ...tail): ...(H, ...T)) {...}
fn foo<H, T: Trait>(head: H, tail: ...T) {...}
fn foo<H, T: Trait>(args: ...(H, ...T)) { let (head, ...tail) = args; ...} |
kennytm
reviewed
Feb 28, 2017
| There is also some unfortunate overlap between the proposed `(head, ...tail)` | ||
| syntax and the current inclusive range syntax. However, the similarity | ||
| between `start...end` and `...tail` can be disambiguiated by whether or not | ||
| there is an expression immediately before the ellipsis. |
This comment has been minimized.
This comment has been minimized.
kennytm
Feb 28, 2017
Member
Sorry but ...x already is a valid inclusive range expression (it creates a std::ops::RangeToInclusive<Tail>). This code works today in nightly:
#![feature(inclusive_range_syntax)]
fn main() {
println!("{:?}", (1, ...(2,)));
// prints `(1, ...(2,))`
}
This comment has been minimized.
This comment has been minimized.
cramertj
force-pushed the
cramertj:variadic-generics
branch
from
de39bfd
to
7be73b6
Feb 28, 2017
This comment has been minimized.
This comment has been minimized.
|
@cramertj There was never such a guarantee, other than by accident - breaking it eventually was always the plan - the problem is the "eventually". |
This comment has been minimized.
This comment has been minimized.
|
@eddyb Yeah, I realize that it's technically undefined behavior, but people still do it. I definitely think it falls under "acceptable breakage." |
aturon
added
the
S-waiting-on-team
label
Jun 6, 2017
This comment has been minimized.
This comment has been minimized.
|
@cramertj Do you think this has reached the point where it'd be useful to discuss in a lang team meeting? |
This comment has been minimized.
This comment has been minimized.
|
@aturon A few of the points I listed above would benefit from some lang team discussion:
I'll also check in @withoutboats again to see if we can come to some consensus WRT their preference for first-class "type lists." |
This comment has been minimized.
This comment has been minimized.
|
@cramertj I don't recall where the |
This comment has been minimized.
This comment has been minimized.
|
@eddyb @sgrif expressed interest in it-- I'm imagining they have some use case in mind. I'd expect the desire is related to implementing a function that takes either a single argument (of type |
This comment has been minimized.
This comment has been minimized.
|
It makes more sense IMO to go straight to variadic functions & methods than have multiple arguments manually packaged in tuples. |
This comment has been minimized.
This comment has been minimized.
|
@eddyb Yes, I've been gradually drifting towards preferring an "all at once" approach myself. My initial goal was to keep this RFC as minimal as possible in the hopes that it would be merged sooner. However, in order to know what the right "first steps" are, you have to consider how they move the language towards a comprehensive solution. |
killercup
referenced this pull request
Jun 27, 2017
Closed
Bug at using tables with more than 16 fields #477
cramertj
referenced this pull request
Aug 28, 2017
Merged
RFC - compiler-generated Clone impls for Arrays and Tuples #2133
This comment has been minimized.
This comment has been minimized.
|
We're clearly not going to get to this before the impl period, so I propose we formally postpone it and start a fresh thread when we're ready to consider it. @rfcbot fcp postpone |
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Sep 6, 2017
•
|
Team member @aturon has proposed to postpone this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
the
proposed-final-comment-period
label
Sep 6, 2017
This comment has been minimized.
This comment has been minimized.
|
Alas, what symmetry that my first ever ticked checkbox as a lang team member should be to postpone my first ever RFC. I'm excited to work more on this next year after we've had a chance to implement the host of wonderful RFCs currently in the queue. |
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Sep 19, 2017
|
|
1 similar comment
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Sep 19, 2017
|
|
rfcbot
added
final-comment-period
and removed
proposed-final-comment-period
labels
Sep 19, 2017
This comment has been minimized.
This comment has been minimized.
|
I'm now closing this RFC as postponed. Thanks, @cramertj and everyone else for pushing further on this issue -- something we'll definitely want to come back to in the future! |
aturon
closed this
Sep 29, 2017
sstangl
added a commit
to sstangl/openpowerlifting
that referenced
this pull request
Oct 5, 2017
Centril
removed
the
S-waiting-on-team
label
Feb 24, 2018
This comment has been minimized.
This comment has been minimized.
alexreg
commented
May 5, 2018
|
Has there been any interest in reviving this lately? |
This comment has been minimized.
This comment has been minimized.
|
@alexreg There's still general interest in the feature, but it's not the kind of change that's targeted for the roadmap this year, so there's probably nothing going on with it right now. (Doing it and const generics could be an interesting thing to consider for next year's roadmap, once we're past the stabilize year.) |
This comment has been minimized.
This comment has been minimized.
alexreg
commented
May 6, 2018
|
@scottmcm Fair enough. w.r.t. Const generics, I thought they were targeted for this year? |
This comment has been minimized.
This comment has been minimized.
|
@alexreg const generics has an RFC merged but implementing it is no official priority of the compiler team. All of the statements were of the form "maybe we can have const generics in nightly by end of 2018"... |
cramertj commentedFeb 28, 2017
Rendered
cc @eddyb @sgrif