Change ManuallyDrop<T> to a lang item. #52711

eddyb · 2018-07-25T18:24:09Z

This PR implements the approach @RalfJung proposes in https://internals.rust-lang.org/t/pre-rfc-unions-drop-types-and-manuallydrop/8025 (lang item struct instead of union).

A followup PR can easily solve #47034 as well, by just adding a few ?Sized to libcore/mem.rs.

r? @nikomatsakis

rust-highfive · 2018-07-25T19:41:56Z

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.

[01:15:50] travis_fold:end:stage0-linkchecker

[01:15:50] travis_time:end:stage0-linkchecker:start=1532547690616675272,finish=1532547693142869704,duration=2526194432

[01:16:03] reference/destructors.html:217: broken link - std/mem/union.ManuallyDrop.html
[01:16:03] reference/print.html:6644: broken link - std/mem/union.ManuallyDrop.html
[01:16:05] thread 'main' panicked at 'found some broken links', tools/linkchecker/main.rs:49:9
[01:16:05] 
[01:16:05] 
[01:16:05] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/linkchecker" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc"
[01:16:05] expected success, got: exit code: 101
[01:16:05] expected success, got: exit code: 101
[01:16:05] 
[01:16:05] 
[01:16:05] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:16:05] Build completed unsuccessfully in 0:30:50
[01:16:05] Makefile:58: recipe for target 'check' failed
[01:16:05] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0e0645c0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:3377eb80:start=1532547710293822346,finish=1532547710303072311,duration=9249965
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:01cea5d2
$ 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:32dd27d6
travis_time:start:32dd27d6
$ 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:0e595516
$ 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)

scottmcm · 2018-07-25T20:15:22Z

src/libcore/mem.rs

-#[derive(Copy)]
-pub union ManuallyDrop<T>{ value: T }
+#[lang = "manually_drop"]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]


Nice that these can all just be derives, now! 👍

nikomatsakis · 2018-07-26T12:35:01Z

@eddyb

[01:16:03] reference/destructors.html:217: broken link - std/mem/union.ManuallyDrop.html
[01:16:03] reference/print.html:6644: broken link - std/mem/union.ManuallyDrop.html

r=me once travis is happy

RalfJung · 2018-07-27T09:04:47Z

src/librustc/traits/query/dropck_outlives.rs

@@ -243,7 +243,10 @@ pub fn trivial_dropck_outlives<'tcx>(tcx: TyCtxt<'_, '_, 'tcx>, ty: Ty<'tcx>) ->

        ty::TyAdt(def, _) => {
            if def.is_union() {
-                // Unions never run have a dtor.
+                // Unions never have a dtor.
+                true


A union implementing Drop itself is not a concern here?

Good question, cc @arielb1 @pnkfelix @nikomatsakis

Actually, yes, I think it is a concern. :D #52786
(but the bug is already on nightly, so it's not introduced by this PR)

RalfJung · 2018-07-27T09:09:50Z

This makes ManuallyDrop for unsized types insta-stable. Does that require an FCP?

@eddyb said he doesn't have much time to follow up on this, but I think I can take over. I should probably also add at least one test because it seems there are none?

eddyb · 2018-07-27T09:50:43Z

FWIW we would only need an FCP for the second commit, the first one can be landed independently and then union-related changes can be made in parallel with generalized to !Sized.

RalfJung · 2018-07-27T09:52:41Z

So shall we split the PR?

RalfJung · 2018-07-27T10:29:25Z

PR submitted to fix link in reference: rust-lang/reference#373

rust-highfive · 2018-07-27T11:31:30Z

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.

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)

RalfJung · 2018-07-27T11:36:23Z

@TimNN Bot shows empty log.

RalfJung · 2018-07-27T11:39:51Z

Updated reference submodule so the links should be fixed.

rust-highfive · 2018-07-27T12:54:04Z

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.

[01:12:44] travis_fold:end:stage0-linkchecker

[01:12:44] travis_time:end:stage0-linkchecker:start=1532696023062930756,finish=1532696025506231937,duration=2443301181

[01:12:56] reference/items/implementations.html:268: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:56] reference/expressions/block-expr.html:205: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:56] reference/print.html:2292: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:56] reference/print.html:3485: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:56] reference/print.html:3845: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:56] reference/statements.html:227: broken link fragment `#lint-check-attributes.html` pointing to `reference/attributes.html`
[01:12:58] thread 'main' panicked at 'found some broken links', tools/linkchecker/main.rs:49:9
[01:12:58] 
[01:12:58] 
[01:12:58] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/linkchecker" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc"
[01:12:58] expected success, got: exit code: 101
[01:12:58] expected success, got: exit code: 101
[01:12:58] 
[01:12:58] 
[01:12:58] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:12:58] Build completed unsuccessfully in 0:30:31
[01:12:58] make: *** [check] Error 1
[01:12:58] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:15946a16
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:03414e70:start=1532696041014582096,finish=1532696041023499098,duration=8917002
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0cac76c6
$ 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:2f984988
travis_time:start:2f984988
$ 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:03449c0e
$ 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)

RalfJung · 2018-07-27T12:55:19Z

Dang, unrelated updates in the reference contain broken links.

Reported as rust-lang/reference#374

EDIT: Fix submitted: rust-lang/reference#375

RalfJung · 2018-07-27T18:00:49Z

r=me once travis is happy

@bors r=nikomatsakis

bors · 2018-07-27T18:00:51Z

📌 Commit 4e6aea1 has been approved by nikomatsakis

bors · 2018-07-28T14:26:23Z

⌛ Testing commit 4e6aea1 with merge 26e73da...

@RalfJung

Change ManuallyDrop<T> to a lang item. This PR implements the approach @RalfJung proposes in https://internals.rust-lang.org/t/pre-rfc-unions-drop-types-and-manuallydrop/8025 (lang item `struct` instead of `union`). A followup PR can easily solve #47034 as well, by just adding a few `?Sized` to `libcore/mem.rs`. r? @nikomatsakis

bors · 2018-07-28T16:37:16Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 26e73da to master...

@eddyb

unsized ManuallyDrop I think this matches what @eddyb had in rust-lang#52711 originally. ~~However, I have never added a `CoerceUnsized` before so I am not sure if I did this right. I copied the `unstable` attribute on the `impl` from elsewhere, but AFAIK it is useless because `impl`'s are insta-stable... so shouldn't this rather say "stable since 1.30"?~~ This is insta-stable and hence requires FCP, at least. Fixes rust-lang#47034

@eddyb

unsized ManuallyDrop I think this matches what @eddyb had in #52711 originally. ~~However, I have never added a `CoerceUnsized` before so I am not sure if I did this right. I copied the `unstable` attribute on the `impl` from elsewhere, but AFAIK it is useless because `impl`'s are insta-stable... so shouldn't this rather say "stable since 1.30"?~~ This is insta-stable and hence requires FCP, at least. Fixes #47034

Change ManuallyDrop from an union to a struct and make it a lang item.

591eeff

rust-highfive assigned nikomatsakis Jul 25, 2018

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

nagisa added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 25, 2018

This comment has been minimized.

Sign in to view

Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jul 25, 2018

scottmcm reviewed Jul 25, 2018

View reviewed changes

eddyb changed the title ~~Change ManuallySized<T> to a lang item and relax it to allow T: ?Sized.~~ Change ManuallyDrop<T> to a lang item and relax it to allow T: ?Sized. Jul 25, 2018

RalfJung reviewed Jul 27, 2018

View reviewed changes

eddyb force-pushed the unsized-manuallydrop branch from f1bf3af to 591eeff Compare July 27, 2018 09:59

eddyb changed the title ~~Change ManuallyDrop<T> to a lang item and relax it to allow T: ?Sized.~~ Change ManuallyDrop<T> to a lang item. Jul 27, 2018

add smoke test for ManuallyDrop

5feedbd

RalfJung mentioned this pull request Jul 27, 2018

fix link to ManuallyDrop rust-lang/reference#373

Merged

update reference to fix links

c9eef35

uodate reference again to hopefully fix all link issues

4e6aea1

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 Jul 27, 2018

bors merged commit 4e6aea1 into rust-lang:master Jul 28, 2018

eddyb deleted the unsized-manuallydrop branch July 28, 2018 19:04

This was referenced Aug 1, 2018

update ManuallyDrop RFC to reflect how the implementation now looks rust-lang/rfcs#2510

Merged

Add empty dropped variant to ManuallyDrop #49056

Closed

unsized ManuallyDrop #53033

Merged

pnkfelix mentioned this pull request Oct 9, 2018

error: internal compiler error: Accessing (*_310) with the kind Write(Move) shouldn't be possible #54597

Closed

bluss mentioned this pull request Dec 3, 2018

Possible UB from use of uninitialized [&T; N] servo/rust-smallvec#126

Closed

RalfJung mentioned this pull request Dec 3, 2018

Implement a stop-gap “MaybeUninit” using ManuallyDrop bluss/arrayvec#97

Closed

RalfJung mentioned this pull request Jul 19, 2019

[BUG] Multiple SIGILL occurences with nightly opt-level=1 amethyst/amethyst#1808

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change ManuallyDrop<T> to a lang item. #52711

Change ManuallyDrop<T> to a lang item. #52711

eddyb commented Jul 25, 2018 •

edited

Loading

This comment has been minimized.

rust-highfive commented Jul 25, 2018

scottmcm Jul 25, 2018

nikomatsakis commented Jul 26, 2018

RalfJung Jul 27, 2018

eddyb Jul 27, 2018

RalfJung Jul 27, 2018 •

edited

Loading

RalfJung commented Jul 27, 2018

eddyb commented Jul 27, 2018

RalfJung commented Jul 27, 2018

RalfJung commented Jul 27, 2018

rust-highfive commented Jul 27, 2018

RalfJung commented Jul 27, 2018

RalfJung commented Jul 27, 2018

rust-highfive commented Jul 27, 2018

RalfJung commented Jul 27, 2018 •

edited

Loading

RalfJung commented Jul 27, 2018

bors commented Jul 27, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

Change ManuallyDrop<T> to a lang item. #52711

Change ManuallyDrop<T> to a lang item. #52711

Conversation

eddyb commented Jul 25, 2018 • edited Loading

This comment has been minimized.

rust-highfive commented Jul 25, 2018

scottmcm Jul 25, 2018

Choose a reason for hiding this comment

nikomatsakis commented Jul 26, 2018

RalfJung Jul 27, 2018

Choose a reason for hiding this comment

eddyb Jul 27, 2018

Choose a reason for hiding this comment

RalfJung Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

RalfJung commented Jul 27, 2018

eddyb commented Jul 27, 2018

RalfJung commented Jul 27, 2018

RalfJung commented Jul 27, 2018

rust-highfive commented Jul 27, 2018

RalfJung commented Jul 27, 2018

RalfJung commented Jul 27, 2018

rust-highfive commented Jul 27, 2018

RalfJung commented Jul 27, 2018 • edited Loading

RalfJung commented Jul 27, 2018

bors commented Jul 27, 2018

bors commented Jul 28, 2018

bors commented Jul 28, 2018

eddyb commented Jul 25, 2018 •

edited

Loading

RalfJung Jul 27, 2018 •

edited

Loading

RalfJung commented Jul 27, 2018 •

edited

Loading