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

Variadic tuples #2775

Open
wants to merge 51 commits into
base: master
from

Conversation

@fredpointzero
Copy link

commented Oct 2, 2019

This RFC introduce variadic tuples, and API to handle tuples with an arbitrary arity.

Rendered
Internal's thread

fredpointzero added 30 commits Aug 16, 2019
- Use same syntax for declaration form and expansion form
- Constraint variadic tuple with same arity in declaration
Separated variadic tuple type and variadic tuple
Added destructuration of variadic tuple
…e impl
@CAD97

This comment has been minimized.

Copy link

commented Oct 2, 2019

Alternatives should at least mention hlist-style expansion of tuple types.

text/0000-variadic-tuples.md Outdated Show resolved Hide resolved

### Expansion

The expansion syntax is: `..<expr(T1, T2, ..., Tn)>` where `<expr(T1, T2, ..., Tn)>` is an expression using the variadic tuple type identifiers `T1`, `T2`, ..., `Tn`.

This comment has been minimized.

Copy link
@ExpHP

ExpHP Oct 2, 2019

"Expression" is a poor choice of term here, because you use it to refer to expressions, patterns and types. The norm would be to specify each of these things differently; there are variadic tuple expressions, variadic tuple types, variadic tuple patterns, and variadic bounds.

text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved

# Rationale and alternatives

[rationale-and-alternatives]: #rationale-and-alternatives

This comment has been minimized.

Copy link
@ExpHP

ExpHP Oct 2, 2019

No mention is made of past proposals like #2702. This needs a section on why this cannot be reasonably implemented as cons-lists. (in particular, why the current design makes it impossible to borrow &(..T) from &(H, ..T)). After all, cons-lists are what basically every single variadic tuple proposal on IRLO has suggested, and I wouldn't be surprised if somebody suggests it within the first four comments on this RFC.

This comment has been minimized.

Copy link
@fredpointzero

fredpointzero Oct 3, 2019

Author

I added a mention to this proposal and I will search for similar proposals. I missed them as I was more focused on variadic generics at the beginning.

I'll take some time to read them and include them into this proposal.

text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
text/0000-variadic-tuples.md Show resolved Hide resolved

# Rationale and alternatives

[rationale-and-alternatives]: #rationale-and-alternatives

This comment has been minimized.

Copy link
@ExpHP

ExpHP Oct 2, 2019

The (for ... @in ...) syntax is also incredibly bizarre and requires incredible justification.

The RFC should explain the grammatical ambiguities that prevent us from simply having (..expr).

This comment has been minimized.

Copy link
@fredpointzero

fredpointzero Oct 3, 2019

Author

I also agree that the syntax is complicated but all the syntaxes I came up had issues and I did not had a lot of feedbacks on them.

So I think I will write sample code with both for loop and (..expr) syntaxes so it will be easier to discuss about it.

@ExpHP

This comment has been minimized.

Copy link

commented Oct 2, 2019

I dropped out of the conversation on internals once people started discussing the for syntax because it was just so bizarre and arcane that I had no idea what people were even saying anymore.

let result: (..Option<&V>) = {
    for (ref k, map) <Key, Value> @in (k, maps) <K, V> {
        HashMap::<Key, Value>::get(&map, k)
    }
};

No matter how many times I see this I cannot for the life of me figure out what it does! It certainly doesn't help that the types of k and maps in this example are not given, nor any sort of desugared result.


The whole thing needs a very thorough proof-read. I opted not to include most grammatical errors in my review because they would have completely swamped the more important bits.

Dynamic libraries are not an issue as machine code for required implementation is embedded in dylibs
@the8472

This comment has been minimized.

Copy link

commented Oct 5, 2019

See also #2702

- Implementations will be easier to read and maintain
- The compiler will compile implementation only for required tuple arity

# Guide-level explanation

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

This… really isn't a very good "guide-level" explanation. It explains everything, but not in the way that "guide-level" explanations merit.

It douses the user with far more detail and jargon than necessary, and it doesn't explain the feature as if it already exists; it explains it as it doesn't, which isn't really what we need.

Will add a bunch of comments on things to improve, but really, I feel like this section needs heavy revision.

text/0000-variadic-tuples.md Outdated Show resolved Hide resolved

Let's call a _variadic tuple type_ a tuple type with an arbitrary arity and a _variadic tuple_ an instance of a variadic tuple type.

A variadic tuple type is declared with `(..T)` and a variadic tuple type can be expanded with `(..Vec<T>)`.

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

Here, "expanded" is really confusing to me. Really, what we're doing is putting constraints on what T actually is.

I'd just say that T represents every type in the tuple, and that we can replace T with something else to further restrict what's in the tuple. For example, T represents any type, Vec<T> represents vectors of any type, usize represents just usize, etc. More examples is generally good here.

This comment has been minimized.

Copy link
@fredpointzero

fredpointzero Oct 7, 2019

Author

I guess this is not clear, because (..Vec<T>) is not a variadic tuple declaration. Only (..T) is allowed.

Maybe I can rephrase it with:

A variadic tuple type is declared with (..T). You can define other variadic tuples types from the idenfier T, for instance (..Vec<T>) defines a variadic tuple type those members are Vecs.

Example:
type TupleOfVec<(..T)> = (..Vec<T>);
TupleOfVec<(uint, bool)> // (Vec<uint>, Vec<bool>)
text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
Variadic tuple types are always declared in a generic parameter group.

There are two different syntaxes:

1. `(..T)`: declare a single variadic tuple type identified by `T`
2. `(..(T1, T2, ..., Tn))`: declare n variadic tuple types identified by `T1`, `T2`, ..., `Tn`, all these variadic tuple types have the same arity.
Comment for lines +73  – +78

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

This is mixing two different explanations in the second example which makes it confusing. Really, what you want is to clarify that ..T anywhere in a tuple refers to the "variadic part," and that T can be replaced by whatever constraint you want to put on the types.

There are 3 syntaxes possible to destructure a variadic tuple for a variadic tuple `(..T)`:

1. `(v @ ..)` of variadic tuple type `(..T)`
2. `(ref v @ ..)` of variadic tuple type `(..&T)`
3. `(ref mut v @ ..)` of variadic tuple type `(..&mut T)`
Comment for lines 156  – 160

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

This honestly isn't the right explanation for this, imho. I think it's mostly just appropriate to say that .. in tuple patterns now can be bound to a variable to get a variadic tuple. You can also bind by reference as need be, or it'll automatically reference into (..&T) or (..&mut T) if the original expression was of type &(..T) or &mut (..T).

This comment has been minimized.

Copy link
@ExpHP

ExpHP Oct 6, 2019

I agree, as this would also help explain why (a, v @ .., b) works.

This comment has been minimized.

Copy link
@fredpointzero

fredpointzero Oct 7, 2019

Author

Indeed, that is way more easier to understand that way.

I updated the section accordingly

text/0000-variadic-tuples.md Outdated Show resolved Hide resolved
We can iterate over the member of a variadic tuple or over the type of a variadic tuple type.

*Important note: the iteration is inlined by the compiler, it is not a generic runtime heterogenous iteration of tuple members.*

We use the following syntax to iterate on variadic tuples:

```rust
// The result of the for block is a variadic tuple made of
// the result of each iteration
let result: (..Option<&V>) = {
// `key` and `map` are variables iterating the variadic tuples `k: (..K)` and `maps: (..&HashMap<K, V>)`, `key` will iterate by reference (because of the ref keyword)
// `KEY` and `VALUE` are type variables iterating the variadic tuple types `(..K)` and `(..V)`
// `(k, maps)` declares the iterated variadic tuples `k` and `maps`
// `<K, V>` declares the iterated variadic tuple types
(for (ref key, map) <KEY, VALUE> @in (k, maps) <K, V> {
HashMap::<KEY, VALUE>::get(&map, key)
})
};
```

Note: when iterating over multiple variadic tuple or variadic tuple types, they must have all the same arity. To ensure this, all variadic tuple types involved must have been declared together.

Examples:

```rust
impl<(..(K, V))> MegaMap<(..(K, V))>
where ..(K: Hash), {
fn get(&self, k: (..K)) -> (..Option<V>) {
let result: (..Option<&V>) = {
(for (ref k, map) <Key, Value> @in (k, &self.maps) <K, V> {
HashMap::<Key, Value>::get(&map, k)
})
};
result
}
}
impl<(..T), Last> Hash for (..T, Last)
where
..(T: Hash),
Last: Hash + ?Sized, {
#[allow(non_snake_case)]
fn hash<S: Hasher>(&self, state: &mut S) {
let (ref tuple @ .., ref last) = *self;
// Use case: only variadic tuple
for member @in tuple {
member.hash(state);
};
last.hash(state);
// Use case: variadic tuple and type
for member <H> @in tuple <T> {
<T as Hash>::hash(&member, state);
};
last.hash(state);
}
}
trait Merge<(..R)> {
type Value;
fn merge(self, r: (..R)) -> Self::Value;
}
impl<(..L), (..R)> Merge<(..R)> for (..L) {
type Value = (..L, ..R);
fn merge(self, r: (..R)) -> Self::Value {
(
for l1 @in self { l1 },
for r1 @in r { r1 },
)
}
}
trait Integer {
fn one() -> Self;
}
fn add_one<(..T)>((..t): (..T)) -> (..T)
where
..(T: Integer + Add), {
(for t1 <T1> @in t <T> { t1 + T1::one() })
}
Comment for lines +202  – +287

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

I really just don't like this syntax. I think that it's 100% fine to leave the initial implementation recursion-only until an iterative syntax is decided upon.

I do agree that an iterative syntax is a good idea, but a for @in loop feels kinda hacky and gross imho, which is the reason why we haven't accepted a variadic tuple syntax yet.

## The `Hash` trait

Let's implement the `Hash` trait:

```rust
// For the example, we consider the impl for (A, B, C). So `(..T)` matches `(A, B, C)`
// We have the first expansion here, `(..T, Last)` expands to `(A, B, C, Last)`
impl<(..T), Last> Hash for (..T, Last)
where
// Expands to `A: Hash, B: Hash, C: Hash,`
..(T: Hash,),
Last: Hash + ?Sized, {
#[allow(non_snake_case)]
fn hash<S: Hasher>(&self, state: &mut S) {
// Destructure self to a variadic tuple `tuple` and a variable `last`. The variadic tuple type of `tuple` is `(..&T)`
// So it will be equivalent to `let (ref a, ref b, ref c, ref last) = *self; let tuple = (a, b, c);`
let (ref tuple @ .., ref last) = *self;
for member @in tuple {
member.hash(state);
};
// The for loop will be inlined as:
// (
// { v.0.hash(state); },
// { v.1.hash(state); },
// { v.2.hash(state); },
// );
last.hash(state);
}
}
```
Comment for lines +290  – +320

This comment has been minimized.

Copy link
@clarfon

clarfon Oct 5, 2019

Contributor

Like I mentioned earlier, an example like this should really be put throughout the section, rather than at the end. Hash is a good example though, and I'd recommend using it.

text/0000-variadic-tuples.md Show resolved Hide resolved
@clarfon

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2019

I left a bunch of feedback on the guide-level explanation. I want to clarify upfront that even though I probably came off as harsh, I really want to help you improve because I do like the core ideas of this syntax. It's a huge undertaking to write an RFC, especially for a big feature like this, so, congratulations on doing that. Even if this RFC itself doesn't get accepted, it will add to the list of inspiration to draw from so we can eventually get a variadic tuple syntax in Rust. :)

@clarfon

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2019

Also: I'm a bit confused why ..T has two dots and (T1, T2, ..., Tn) has three dots. Also, shouldn't the latter be (T1, T2, ...Ti, Tn) ? Also, is (...) ever a valid syntax?

@ExpHP

This comment has been minimized.

Copy link

commented Oct 6, 2019

I don't think the ... is a literal ... token...

That is,

(..(T1, T2, ..., Tn))

means

(..(T1,))
OR (..(T1, T2))
OR (..(T1, T2, T3))
OR (..(T1, T2, T3, T4))
...
@clarfon

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2019

OH. I understand now. I was confusing syntax for notation.

@fredpointzero

This comment has been minimized.

Copy link
Author

commented Oct 7, 2019

A small update concerning the feedbacks received. First thanks for your feedbacks, I will take them into account, as they are a lot of them I will process them over several days.

Cons-list

As suggested I looked at other RFCs and focused mostly on cons-list proposals (like #2702).

To do so, I wrote some use case with both RFCs (https://gist.github.com/fredpointzero/7b40e93d3e2226ca4aec00fdd1bc3e20).

Note: I used a less gross syntax for the variadic tuple RFC, by using

for (k, map), (Key, Value) in (&k, &self.maps), (K, V) {
    HashMap::<Key, Value>::get(&map, k)
}

// Instead of
for (k, map) <Key, Value> @in (&k, &self.maps) <K, V> {
    HashMap::<Key, Value>::get(&map, k)
}

What I found is:

  1. The RFC I suggested includes cons-list as well. (See Rev implementation) so this is as expressive as cons-list proposals
  2. A lot of implementations can be implemented only with cons-lists (See Hash, Merge, Zip, ...)
  3. Some implementations can't be implemented or are very difficult to implement with only cons-lists (See MegaMap, Split)
  4. This RFC is probably more complex to implement in Rust language than other proposals

So, in my opinion, both cons-list and iterative patterns must be available to use when considering variadic tuples.

I haven't looked at hlist yet, but I will do later.

Clarity of the RFC

The current document is not clear and I will first focus on making it easier to understand. Mostly the guide-level section.

For loop syntax

I am not fan of the syntax as well, and still will try to come up with something better.

A little note here, although the feature is powerfull enough without the variadic tuple iteration, I prefer to make sure that the variadic tuple iteration is possible, at least in future possibilities. In my opinion, this is required to implementent some features easily.

And a quick suggestion here, to have something less gross we can use the syntax suggested above.
Or also, as the issue is about iterating through two kinds of variable (variadic tuple members and variadic tuple type members) we may use two for loops, like:

for (k, map) in (&k, &self.maps)		// variadic tuple
for (Key, Value) in (K, V) {			// variadic tuple type
    HashMap::<Key, Value>::get(&map, k)
}

// Instead of
for (k, map) <Key, Value> @in (&k, &self.maps) <K, V> {
    HashMap::<Key, Value>::get(&map, k)
}
let (head, tail @ ..): (Head, (..Tail)) = source;
let (head, tail @ ..): (&Head, (..&Tail)) = &source;
let (head, tail @ ..): (&mut Head, (..&mut Tail)) = &mut source;
Comment for lines +225  – +227

This comment has been minimized.

Copy link
@the8472

the8472 Oct 13, 2019

Is &Tail a reference to the tail or a tuple of references? The former would prevent currently performed struct layout optimizations, see rust-lang/unsafe-code-guidelines#105 for more details. The latter would be costly since could require a lot of intermediate values on the stack.

This comment has been minimized.

Copy link
@KrishnaSannasi

KrishnaSannasi Oct 13, 2019

It's a tuple of references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.