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

all but the last field of a tuple must be Sized #1592

Merged
merged 1 commit into from
May 13, 2016

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Apr 27, 2016

This oversight was found while working on rust-lang/rust#33138.

Tuples with an unsized field in the middle can't possibly work. Tuples with an unsized field at the end should work after we refactor trans::adt.

@nikomatsakis
Copy link
Contributor

@arielb1 looks good, can you add an "Edit history" near the end with an entry like

@@ -719,7 +719,8 @@ declare one), but we'll take those basic conditions for granted.

WfTuple:
∀i. R ⊢ Ti WF
--------------------------------------------------
∀i<n. R ⊢ Ti: Sized // the *last* field may be unsized
--------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation on this inference rule seems to have been lost

@arielb1
Copy link
Contributor Author

arielb1 commented Apr 27, 2016

@ticki

This is already the requirement for structs. This RFC PR is fixing that oversight.

@ticki
Copy link
Contributor

ticki commented Apr 27, 2016

Yeah, I realized that, which is why I deleted the comment.

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Apr 27, 2016
@nikomatsakis nikomatsakis self-assigned this Apr 28, 2016
@nikomatsakis nikomatsakis added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label May 2, 2016
@nikomatsakis
Copy link
Contributor

Hear ye, hear ye! This RFC is now entering final comment period.

@bluss
Copy link
Member

bluss commented May 7, 2016

I'm repeating myself, but this RFC seems to conflict with the representation ideas in /pull/1582, what do you think about that?

@arielb1
Copy link
Contributor Author

arielb1 commented May 7, 2016

The problem is that the ill-formed tuples cause ICEs.

@canndrew
Copy link
Contributor

canndrew commented May 9, 2016

If we ever wanted to go with the tuple representation proposed in #1582 then this would need to be "all but the first field of a tuple must be Sized". The alternatives are:

  • Change that RFC so that tuples are split into a head tuple and a tail element instead of a head element and a tail tuple. I don't what the syntax for this could look like, maybe (head..., tail) but that looks confusingly like range syntax.
  • Go with the alternative representation in the RFC, where tuples are packed less efficiently.

@bluss
Copy link
Member

bluss commented May 9, 2016

@arielb1 I mean that there are Rfcs that propose alternate tuple representations that directly conflict with allowing the last field be unsized, at least it is a conflict as I understand it. That needs to be addressed explicitly.

For example, you could say it is best to have tuple fields be laid out in order with a possible flexible size field last.

@nikomatsakis
Copy link
Contributor

@bluss sorry, for the record can you summarize or give some pointers to the conflict? One problem I am aware of is that, if you try to use tuple types as the basis for "variadic generics", that would require tuple types that can permit multiple unsized values to be stored within them (but I don't think this is really tied to the representation per se. That is, the same problem arises regardless of how tuples are laid out in memory.) I've not been able to follow the other RFC thread in great detail, so I may not be caught up on the latest complications there.

@arielb1 the fact that ICEs are caused doesn't seem, in and of itself, to mean much. =) It's more interesting to dig into the causes of those ICEs. Perhaps you can say more about that?

There is definitely an inherent inconsistent in allowing tuples to have multiple unsized values but not structs, but perhaps a reconcilable one. I could also imagine some rules that say that a tuple type may contain multiple unsized values, but that such a type could not be used in most contexts, but it seems kind of complicated.

In any case, I don't necessarily think that THIS RFC amendment is the right place to resolve this conflict. That is, I think that this RFC is (as @arielb1 suggested) adding something that ought to have been in the initial RFC to begin with. Rather, the onus is on later RFCs to make the WF rules for tuples more liberal and work out the conflicts that arise as a result (I believe such a change would be backwards compatible).

@strega-nil
Copy link

strega-nil commented May 10, 2016

@nikomatsakis Why would using tuple types as the basis for variadic generics require permitting multiple unsized values? As far as @eddyb and I could tell, they just required some compiler trait implementation support and they were fine. You can't actually pass unsized types.

Note: incorrect assumptions. It's to do with being able to map over types, as someone explained to me.

@arielb1
Copy link
Contributor Author

arielb1 commented May 10, 2016

Causes ICEs = the tuple can't be represented, so unless we want to introduce a new Representable bound or something, an Rc<([u8], [u8])> is impossible to codegen.

@canndrew
Copy link
Contributor

@nikomatsakis RFC #1582 proposes being able to write tuples as a head-element, tail-tuple pair like so: (head; tail). This enables being able to do induction over tuples so that you can do things like impl Foo for () and impl<H, T: Tuple> Foo for (H; T) to impl Foo for all tuples. This requires being able to take a reference to the tail tuple of a larger tuple: let (_; ref tail) = (0, 1, 2);. This puts restrictions on the representation of tuples - the representation of a tuple must always contain the representation of its tail. The two ways to acheive this are to (a) pack tuples less efficiently with lots of padding or (b) reverse the layout order (so that the syntactically last element is first in memory) and adopt #1397 so that a tuple's head element can be placed in the trailing padding of its tail. (b) would disallow the last element of a tuple being !Sized and so conflicts with this RFC.

If we take a few steps back we can fix this problem by changing the cons-cell syntax so that the n - 1 elements of an n element tuple are grouped into the tuple's prefix rather than the suffix. That would allow us to go with the efficient tuple representation while allowing the syntactically last element of a tuple to be placed at the end of the representation (assuming #1397). This seems to me to be the better option. The only problem left is that we'd need syntax for it. I don't think the semicolon-delimiter is quite as pretty this way around, 3-dots has been suggested (head..., tail) but that looks too much like inclusive range syntax.

Also I think it's an abuse of tuples to use them as lists of types. ([u8], [u8]) isn't a valid type. If we ever need a way to represent arbitrary lists of types for the sake of variadic generics or whatever we should either avoid having syntax like ([u8], [u8]) or make it clear that just because something is written (a, b) doesn't mean it's a tuple.

@glaebhoerl
Copy link
Contributor

The current situation: none of the fields of a tuple are required to be Sized.

What this RFC proposes: require the first N-1 fields of a tuple to be Sized.

What's up for debate w.r.t. #1582: should we also require the last field of a tuple to be Sized?

So I don't think this RFC and #1582 are in conflict.

@bluss
Copy link
Member

bluss commented May 11, 2016

@glaebhoerl this RFC also further reinforces that the last field of the tuple is allowed to be ?Sized and that is a technical conflict with a reversed tuple representation.

I was thinking of the simple case where the representation's last field is unsized. This RFC would in that sense mandate that tuples are explicitly ordered so that the logically last field is also last in the actual representation. That's a technical conflict with the proposal for "tuple slicing" in the other RFC which canndrew describes in this thread. I don't think that's a big deal, but it just needs some coordination or acknowledgement if you rule out an idea that is being developed in parallel.


I think the current order of fields in the tuple representation is better, and that "tuple slicing" should instead destructure them into (initial_part.., last). Keeping the tuple field order makes it possible to have (T, T) and [T; 2] representation compatible, which is something that rustc already uses and that other projects would like to be able to use if it were a documented guarantee (numerical crates I believe).

@glaebhoerl
Copy link
Contributor

glaebhoerl commented May 11, 2016

That is definitely something which should be debated and decided, but I don't see why it needs to hold up fixing the obvious bug that currently the first N-1 fields are allowed to be !Sized.

Maybe tweak the wording of the RFC text with respect to the last field if it's currently too definitive. But I don't think this should stop us from moving forward on the part that everyone agrees about. The debate about the last field can continue on #1582 which I think is a better place for it anyways.

@canndrew
Copy link
Contributor

You're right, sorry, I was just worried that this RFC would solidify allowing the last field to be !Sized but #1582 should probably be changed to accommodate that anyway.

@nikomatsakis
Copy link
Contributor

Yes, good point -- the reason is because of my desire to allow passing
unsized types by value as fn arguments (and in certain other contexts where
we can do it safely).

On Tue, May 10, 2016 at 07:01:17AM -0700, Nicole Mazzuca wrote:

@nikomatsakis Why would using tuple types as the basis for variadic generics require permitting multiple unsized values? As far as @eddyb and I could tell, they just required some compiler trait implementation support and they were fine. You can't actually pass unsized types.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1592 (comment)

@nikomatsakis
Copy link
Contributor

I am, I admit, a bit torn. On the one hand, I think that this RFC
is codifying the most natural extension of the WF RFC to tuples.

On the other, if we did wind up wanting some change here that required
that a tuple's last elem be Sized, it'd be backwards incompatible.
(On the gripping hand, I can't imagine agreeing to such a change,
which strikes me as quite surprising.)

But of course in practice it's all quite moot since the compiler
hasn't implemented much support for tuples with unsized types in
them. For example, the Unsize coercion traits are not implemented
for tuples, and if you try to write code you
can easily encounter ICEs.

So this is leading me to wonder if we should (for now) say that tuples
must consist only of sized types, which I think would be a backwards
compatible state from which we can relax the rules accordingly, and
would reflect compiler state.

On Wed, May 11, 2016 at 06:46:37AM -0700, Gábor Lehel wrote:

That is definitely something which should be debated and decided, but I don't see why it needs to hold up fixing the obvious bug that currently the first N-1 fields are allowed to be !Sized.

Maybe tweak the wording of the RFC text with respect to the last field if it's currently too definitive. But I don't think it should stop us from moving forward on the part that everyone agrees about. The debate about the last field can continue on #1582 which I think is a better place for it anyways.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1592 (comment)

@strega-nil
Copy link

@nikomatsakis I don't think that passing an unsized type by value is a useful enough extension to the language to change tuples from being tuple struct like (over owned stack pointers); in fact, I haven't seen any change that makes me think that tuples should be any different from tuple structs.

@strega-nil
Copy link

(that is useful enough to complicate the language into making tuples more than just an anonymous struct type)

@nikomatsakis
Copy link
Contributor

@ubsan

I don't think that passing an unsized type by value is a useful enough extension to the language to change tuples from being tuple struct like (over owned stack pointers)

Well, I think this is quite unclear, in two respects. First, with respect to general utility, I consider passing unsized types by value a very useful extension to the language (and closing an unnecessary lack of symmetry). But in any case it's not a question that must be settled at this time.

The second question is whether that would require making tuples be different from structs. One could imagine variadic generic systems that are not based on tuples.

@strega-nil
Copy link

@nikomatsakis Allowing passing DSTs as function parameters is pretty much the only place where we could allow it; elsewhere, unsized types are not allowed (for example, in returns). It would open an unnecessary lack of symmetry in the language, IMO.

@golddranks
Copy link

@ubsan Couldn't DST returns, in principle, be allowed with the placement syntax?

@nikomatsakis
Copy link
Contributor

Huzzah! The @rust-lang/lang team has decided to accept this RFC. We debated going further (e.g., requiring that tuples consist purely of sized values) in the interest of maximum compatibility, but decided in the end that it was better to take the RFC as is, since it represents the most natural extension of the existing RFC. (It's still the case though that tuples with unsized values are not well supported by the existing compiler.)

@nikomatsakis nikomatsakis merged commit b2c8577 into rust-lang:master May 13, 2016
@Centril Centril added A-typesystem Type system related proposals & ideas A-machine Proposals relating to Rust's abstract machine. A-repr #[repr(...)] related proposals & ideas labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-machine Proposals relating to Rust's abstract machine. A-repr #[repr(...)] related proposals & ideas A-typesystem Type system related proposals & ideas final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.