Unsized tuple coercions #42527

qnighy · 2017-06-08T06:09:54Z

Part of #18469. Fixes #32702.

#37685 and #34451 might also be related.

This PR does the following:

Introduce explicit Sized constraints on tuple initializers, similar to that of record-struct initializers. Not much relevant to the main contribution but I noticed this when making tests for unsized tuple coercions.
Implement (.., T): Unsize<(.., U)> where T: Unsize<U>.
Assume (.., T) is MaybeUnsizedUnivariant.
Modify src/librustc/ty/util.rs and src/librustc_trans/glue.rs so that tuples and structs are uniformly traversed when translating.

rust-highfive · 2017-06-08T06:09:59Z

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

qnighy · 2017-06-08T08:06:11Z

Travis failed, but I suppose it is spurious.

oli-obk · 2017-06-08T09:58:01Z

You can close and reopen the PR to restart travis

qnighy · 2017-06-08T13:18:40Z

@oli-obk Thank you!

carols10cents · 2017-06-12T12:30:38Z

Friendly ping @arielb1 to keep this on your radar!

arielb1 · 2017-06-12T18:55:57Z

Don't we want to feature-gate this somehow? Pinging @rust-lang/lang.

arielb1 · 2017-06-12T19:39:45Z

src/librustc/traits/select.rs

@@ -2612,6 +2617,37 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                    &[inner_target]));
            }

+            // (.., T) -> (.., U).
+            (&ty::TyTuple(tys_a, _), &ty::TyTuple(tys_b, _)) => {


I'll prefer to merge this with the ADT code somehow.

I've thought before that we should have an "infinite family" of tuple ADTs. One complication is that ADTs have def-ids...

The code above mk_adt largely depends on def-ids and type parameters, so I doubt I can merge this part with tuple's. The codes below mk_adt/mk_tup are similar, but I don't think its worth merging...

arielb1 · 2017-06-12T19:43:27Z

src/test/compile-fail/dst-bad-assign-3.rs

+    f: isize
+}
+
+trait ToBar {


Why these extra methods?

Just because it was copied from dst-bad-assign.rs.

arielb1 · 2017-06-12T19:45:54Z

src/test/run-pass-valgrind/dst-dtor-1.rs

@@ -28,7 +28,8 @@ struct Fat<T: ?Sized> {

 pub fn main() {
    {
-        let _x: Box<Fat<Trait>> = Box::<Fat<Foo>>::new(Fat { f: Foo });
+        let _x: Box<(i32, Fat<Trait>)> =


can you have this in addition to the original test?

arielb1 · 2017-06-12T19:46:01Z

src/test/run-pass-valgrind/dst-dtor-2.rs

@@ -25,7 +25,8 @@ struct Fat<T: ?Sized> {

 pub fn main() {
    {
-        let _x: Box<Fat<[Foo]>> = Box::<Fat<[Foo; 3]>>::new(Fat { f: [Foo, Foo, Foo] });
+        let _x: Box<(Fat<[Foo]>,)> =


can you have this in addition to the original test?

arielb1 · 2017-06-12T19:47:43Z

I'm generally ok with these changes modulo select cleanup and a feature gate.

arielb1 · 2017-06-20T20:30:58Z

Hi @qnighy. We don't want this change to be insta-stable, so we need to add a feature gate and a tracking issue.

arielb1 · 2017-06-28T09:20:51Z

r+ modulo comment

arielb1

r+ modulo comment. Nice PR!

arielb1 · 2017-06-28T09:10:27Z

src/librustc/traits/select.rs

+                let b_last = tys_b.last().unwrap();
+
+                // Check that the source tuple with the target's
+                // last element is a subtype of the target.


nit: comment should not say "subtype" but rather "has the same type". Yes I know this is a copy of another comment that says "subtype". Both are wrong. The code does a type equality, and subtyping would be unsound (#40288).

So this comment should be

// Check that the source tuple with the target's // last element is equal to the target.

And the other one

// Check that the source struct with the target's // unsized parameters equal to the target.

We should probably be comparing all other type parameters instead of doing this hack. Feel free to do that (in both places), but I'm not going to block this PR on that.

Fixed, thanks!

arielb1 · 2017-06-28T09:19:12Z

src/librustc_typeck/check/coercion.rs

        // Keep resolving `CoerceUnsized` and `Unsize` predicates to avoid
        // emitting a coercion in cases like `Foo<$1>` -> `Foo<$2>`, where
        // inference might unify those two inner type variables later.
        let traits = [coerce_unsized_did, unsize_did];
        while let Some(obligation) = queue.pop_front() {
            debug!("coerce_unsized resolve step: {:?}", obligation);
            let trait_ref = match obligation.predicate {
-                ty::Predicate::Trait(ref tr) if traits.contains(&tr.def_id()) => tr.clone(),
+                ty::Predicate::Trait(ref tr) if traits.contains(&tr.def_id()) => {


this is... ok.

bors · 2017-06-28T22:43:45Z

☔ The latest upstream changes (presumably #42850) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2017-06-29T11:14:18Z

☔ The latest upstream changes (presumably #42964) made this pull request unmergeable. Please resolve the merge conflicts.

arielb1 · 2017-06-29T13:57:16Z

@bors r+

bors · 2017-06-29T13:57:17Z

📌 Commit 94862c6 has been approved by arielb1

bors · 2017-06-29T15:04:37Z

⌛ Testing commit 94862c6 with merge 686ec28...

Unsized tuple coercions Part of #18469. Fixes #32702. #37685 and #34451 might also be related. This PR does the following: - Introduce explicit `Sized` constraints on tuple initializers, similar to that of record-struct initializers. Not much relevant to the main contribution but I noticed this when making tests for unsized tuple coercions. - Implement `(.., T): Unsize<(.., U)>` where `T: Unsize<U>`. - Assume `(.., T)` is MaybeUnsizedUnivariant. - Modify `src/librustc/ty/util.rs` and `src/librustc_trans/glue.rs` so that tuples and structs are uniformly traversed when translating.

bors · 2017-06-29T18:23:19Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 686ec28 to master...

Implement Eq/Hash/Debug etc. for unsized tuples. As I mentioned in [the comment in rust-lang#18469](rust-lang#18469 (comment)), the implementations of `PartialEq`, `Eq`, `PartialOrd`, `Ord`, `Debug`, `Hash` can be generalized to unsized tuples. This is consistent with the `derive` behavior for unsized structs. ```rust #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default, Hash)] struct MyTuple<X, Y, Z: ?Sized>(X, Y, Z); fn f(x: &MyTuple<i32, i32, [i32]>) { x == x; x < x; println!("{:?}", x); } ``` Questions: - Need an RFC? - Need a feature gate? I don't think it does because the unsized tuple coercion rust-lang#42527 is feature-gated. - I changed `builder.field($name);` into `builder.field(&$name);` in the `Debug` implementation to pass compilation. This won't affect the behavior because `Debug for &'a T` is a mere redirection to `Debug for T`. However, I don't know if it affects code size / performance.

rust-highfive assigned arielb1 Jun 8, 2017

qnighy closed this Jun 8, 2017

qnighy reopened this Jun 8, 2017

alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 8, 2017

arielb1 reviewed Jun 12, 2017

View reviewed changes

arielb1 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2017

qnighy force-pushed the unsized-tuple-coercions branch from 8722ad7 to 818f416 Compare June 24, 2017 07:28

qnighy mentioned this pull request Jun 24, 2017

Tracking issue for unsized tuple coercion #42877

Open

arielb1 suggested changes Jun 28, 2017

View reviewed changes

qnighy force-pushed the unsized-tuple-coercions branch from 1fe43a5 to 0812b33 Compare June 29, 2017 11:08

Check explicitly that tuple initializer is Sized.

17fca8b

qnighy added 5 commits June 29, 2017 21:23

Add unsized tuple coercions.

23d1521

Split dst-dtor-{1,2} tests into four.

b0bf1b4

Move unsized_tuple_coercion behind a feature gate.

03660b6

Give a tracking-issue number for unsized tuple coercion.

141265d

Correct comments about unsizing regarding rust-lang#40319.

94862c6

qnighy force-pushed the unsized-tuple-coercions branch from 0812b33 to 94862c6 Compare June 29, 2017 12:26

arielb1 approved these changes Jun 29, 2017

View reviewed changes

bors merged commit 94862c6 into rust-lang:master Jun 29, 2017

qnighy deleted the unsized-tuple-coercions branch June 30, 2017 16:32

qnighy mentioned this pull request Jul 2, 2017

Implement Eq/Hash/Debug etc. for unsized tuples. #43011

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unsized tuple coercions #42527

Unsized tuple coercions #42527

qnighy commented Jun 8, 2017

rust-highfive commented Jun 8, 2017

qnighy commented Jun 8, 2017

oli-obk commented Jun 8, 2017

qnighy commented Jun 8, 2017

carols10cents commented Jun 12, 2017

arielb1 commented Jun 12, 2017

arielb1 Jun 12, 2017

nikomatsakis Jun 13, 2017

qnighy Jun 14, 2017

arielb1 Jun 12, 2017

qnighy Jun 14, 2017

arielb1 Jun 12, 2017

qnighy Jun 14, 2017

arielb1 Jun 12, 2017

qnighy Jun 14, 2017

arielb1 commented Jun 12, 2017

arielb1 commented Jun 20, 2017

arielb1 commented Jun 28, 2017

arielb1 left a comment

arielb1 Jun 28, 2017

qnighy Jun 29, 2017

arielb1 Jun 28, 2017

bors commented Jun 28, 2017

bors commented Jun 29, 2017

arielb1 commented Jun 29, 2017

bors commented Jun 29, 2017

bors commented Jun 29, 2017

bors commented Jun 29, 2017

Unsized tuple coercions #42527

Unsized tuple coercions #42527

Conversation

qnighy commented Jun 8, 2017

rust-highfive commented Jun 8, 2017

qnighy commented Jun 8, 2017

oli-obk commented Jun 8, 2017

qnighy commented Jun 8, 2017

carols10cents commented Jun 12, 2017

arielb1 commented Jun 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 commented Jun 12, 2017

arielb1 commented Jun 20, 2017

arielb1 commented Jun 28, 2017

arielb1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jun 28, 2017

bors commented Jun 29, 2017

arielb1 commented Jun 29, 2017

bors commented Jun 29, 2017

bors commented Jun 29, 2017

bors commented Jun 29, 2017