Implement Unpin for Box, Rc, and Arc #53874

withoutboats · 2018-08-31T23:56:17Z

Per the discussion in #49150, these should implement Unpin even if what they point to does not.

rust-highfive · 2018-08-31T23:56:20Z

r? @KodrAus

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

withoutboats · 2018-08-31T23:56:53Z

@bors r? @RalfJung

cc @cramertj @carllerche

rust-highfive · 2018-09-01T00:03:10Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/d7/d2/04d4fb0da66c7a8850477ff1fb8f9e80597147286e118e111f7d548cf1d7/awscli-1.16.6-py2.py3-none-any.whl (1.3MB)
    0% |▎                               | 10kB 13.9MB/s eta 0:00:01
    1% |▌                               | 20kB 1.9MB/s eta 0:00:01
    2% |▊                               | 30kB 2.2MB/s eta 0:00:01
    3% |█                               | 40kB 2.0MB/s eta 0:00:01

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

seanmonstar · 2018-09-01T00:32:31Z

src/liballoc/rc.rs

@@ -1830,3 +1830,6 @@ impl<T: ?Sized> AsRef<T> for Rc<T> {
        &**self
    }
 }
+
+#[unstable(feature = "pin", issue = "49150")]
+impl<T: ?Sized> Unpin for Box<T> { }


This looks like a copy-paste woops, it should be Rc, right? XD

yep whoops!

seanmonstar · 2018-09-01T00:34:12Z

src/liballoc/boxed.rs

@@ -749,6 +749,9 @@ impl<T: ?Sized> AsMut<T> for Box<T> {
    }
 }

+#[unstable(feature = "pin", issue = "49150")]
+impl<T: ?Sized> Unpin for Box<T> { }


I tried looking through the issue, and it wasn't immediately clear: why is a Box unpin? After using a pin, we could move out of the box afterwards, right?

Best summary is this comment under "adding Unpin implementations" TL;DR is you're right, but putting the box in the pin doesn't pin the stuff in the heap, only the box's stack representation (i.e. the actual pointer)

Both interpretations of Box are valid, but this one is the most useful and consistent with other types (in that pin-projection is related to whether or not the data is stored out-of-line). If a Box is pinned, then there isn't a way to move the underlying data unless we choose to make it Unpin, so the other choice is fine as well.

RalfJung · 2018-09-01T10:12:47Z

LGTM, but I feel those arguments about Box possibly being both ways, and about pinning being structural for fields but not for pointer indirections, should also be written down in comments somewhere in the code.

r=me with that comment added.

withoutboats · 2018-09-05T21:48:37Z

@bors r=RalfJung

bors · 2018-09-05T21:48:38Z

📌 Commit c82af09 has been approved by RalfJung

rust-highfive · 2018-09-05T22:04:16Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.


[00:06:50] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:06:50] tidy error: /checkout/src/liballoc/boxed.rs:754: trailing whitespace
[00:06:52] some tidy checks failed
[00:06:52] 
[00:06:52] 
[00:06:52] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:06:52] 
[00:06:52] 
[00:06:52] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:06:52] Build completed unsuccessfully in 0:00:51
[00:06:52] Build completed unsuccessfully in 0:00:51
[00:06:52] make: *** [tidy] Error 1
[00:06:52] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:090e55ea
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:14559ba0:start=1536184653303742405,finish=1536184653312308198,duration=8565793
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:16a90cbb
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:383c0220
travis_time:start:383c0220
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2f34c2f1
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

ghost · 2018-09-05T23:31:08Z

src/liballoc/boxed.rs

+ *        standard library pointer types support projecting through a pin
+ *        (Box<T> is the only pointer type in std for which this would be
+ *        safe.)
+ *      - It is in practive very useful to have Box<T> be unconditionally


Nit: s/practive/practice/

kennytm · 2018-09-06T05:27:45Z

src/liballoc/boxed.rs

@@ -749,6 +749,31 @@ impl<T: ?Sized> AsMut<T> for Box<T> {
    }
 }

+/* Nota bene
+ *
+ *  We could have chosen not to add this impl, and instead have written a 


Trailing whitespace fails tidy

kennytm · 2018-09-06T05:27:58Z

@bors r-

Tidy failed.

withoutboats · 2018-09-06T19:32:15Z

@bors r=RalfJung

bors · 2018-09-06T19:32:16Z

📌 Commit 972cd8b has been approved by RalfJung

kennytm · 2018-09-07T05:39:12Z

@bors rollup

Implement Unpin for Box, Rc, and Arc Per the discussion in rust-lang#49150, these should implement `Unpin` even if what they point to does not.

Rollup of 17 pull requests Successful merges: - #53299 (Updated core/macros.rs to note it works in a no_std environment.) - #53376 (Cross reference io::copy and fs::copy in docs.) - #53455 (Individual docs for {from,to}_*_bytes) - #53550 (librustc_lint: In recursion warning, change 'recurring' to 'recursing') - #53860 (Migrate (some) of run-pass/ to ui) - #53874 (Implement Unpin for Box, Rc, and Arc) - #53895 (tidy: Cleanups and clippy warning fixes) - #53946 (Clarify `ManuallyDrop` docs) - #53948 (Minimized clippy test from when NLL disabled two-phase borrows) - #53959 (Add .git extension to submodule paths missing it) - #53966 (A few cleanups and minor improvements to mir/dataflow) - #53967 (propagate build.python into cmake) - #53979 (Remove `#[repr(transparent)]` from atomics) - #53991 (Add unchecked_shl/shr check for intrinsics to fix miri's test suit) - #53992 (migrate run-pass/borrowck to ui/run-pass) - #53994 (migrate run-pass/*/ to ui/run-pass) - #54023 (update clippy submodule)

Implement Unpin for Box, Rc, and Arc

c3bdd76

rust-highfive assigned KodrAus Aug 31, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2018

rust-highfive assigned RalfJung and unassigned KodrAus Aug 31, 2018

carllerche mentioned this pull request Sep 1, 2018

[tokio-async-await] Stream support tokio-rs/tokio#595

Closed

seanmonstar reviewed Sep 1, 2018

View reviewed changes

Fix Rc impl.

9ff29d6

Add comment.

c82af09

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2018

ghost reviewed Sep 5, 2018

View reviewed changes

kennytm reviewed Sep 6, 2018

View reviewed changes

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 6, 2018

Fix typos.

972cd8b

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 2018

kennytm mentioned this pull request Sep 7, 2018

Rollup of 17 pull requests #54021

Merged

bors merged commit 972cd8b into rust-lang:master Sep 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Unpin for Box, Rc, and Arc #53874

Implement Unpin for Box, Rc, and Arc #53874

withoutboats commented Aug 31, 2018

rust-highfive commented Aug 31, 2018

withoutboats commented Aug 31, 2018

rust-highfive commented Sep 1, 2018

seanmonstar Sep 1, 2018

withoutboats Sep 1, 2018

seanmonstar Sep 1, 2018

withoutboats Sep 1, 2018

cramertj Sep 1, 2018 •

edited

Loading

RalfJung commented Sep 1, 2018

withoutboats commented Sep 5, 2018

bors commented Sep 5, 2018

rust-highfive commented Sep 5, 2018

ghost Sep 5, 2018

kennytm Sep 6, 2018

kennytm commented Sep 6, 2018

withoutboats commented Sep 6, 2018

bors commented Sep 6, 2018

kennytm commented Sep 7, 2018

Implement Unpin for Box, Rc, and Arc #53874

Implement Unpin for Box, Rc, and Arc #53874

Conversation

withoutboats commented Aug 31, 2018

rust-highfive commented Aug 31, 2018

withoutboats commented Aug 31, 2018

rust-highfive commented Sep 1, 2018

seanmonstar Sep 1, 2018

Choose a reason for hiding this comment

withoutboats Sep 1, 2018

Choose a reason for hiding this comment

seanmonstar Sep 1, 2018

Choose a reason for hiding this comment

withoutboats Sep 1, 2018

Choose a reason for hiding this comment

cramertj Sep 1, 2018 • edited Loading

Choose a reason for hiding this comment

RalfJung commented Sep 1, 2018

withoutboats commented Sep 5, 2018

bors commented Sep 5, 2018

rust-highfive commented Sep 5, 2018

ghost Sep 5, 2018

Choose a reason for hiding this comment

kennytm Sep 6, 2018

Choose a reason for hiding this comment

kennytm commented Sep 6, 2018

withoutboats commented Sep 6, 2018

bors commented Sep 6, 2018

kennytm commented Sep 7, 2018

cramertj Sep 1, 2018 •

edited

Loading