Skip to content

error message when trying to move from an Rc or Arc is ungreat #54703

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 5, 2018
Merged

error message when trying to move from an Rc or Arc is ungreat #54703

merged 2 commits into from
Oct 5, 2018

Conversation

davidtwco
Copy link
Member

Fixes #52086.

r? @nikomatsakis

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

This comment has been minimized.

Sign in to view
@tmandry
Copy link
Member

tmandry commented Oct 1, 2018

Nice! Just curious, was there any discussion of fixing this for smart pointer types other than Rc and Arc? (i.e. any type that used Deref along the path)

I have no idea if this is possible. This clearly fixes the most important error messages, but wonder if it could be done more generically at some point.

@davidtwco
Introduce language items for Arc and Rc.
da4a120 
This commit introduces language items for `Arc` and `Rc` so that types
can later be checked to be `Arc` or `Rc` in the NLL borrow checker. The
`lang` attribute is currently limited to `stage1` as it requires a
compiler built with knowledge of the expected language items.
@davidtwco
Add special cases for move from Rc/Arc errors.
8c6d08b 
This commit special cases the move out of borrowed content error,
previously:

```
error[E0507]: cannot move out of borrowed content
 --> src/main.rs:7:10
  |
7 |     drop(x.field);
  |          ^ cannot move out of borrowed content
```

to instead mention that it is a move out of a `Rc`/`Arc` which is more
helpful:

```
error[E0507]: cannot move out of an `Rc`
 --> src/main.rs:7:10
  |
7 |     drop(x.field);
  |          ^ cannot move out of an `Rc`
```
nikomatsakis
nikomatsakis reviewed Oct 1, 2018
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Thoughts?

nikomatsakis
nikomatsakis requested changes Oct 2, 2018
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Thoughts?

@nikomatsakis
Copy link
Contributor

(I'm ok with starting with the Rc-Arc error for now)

@davidtwco
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Oct 3, 2018

📌 Commit 8c6d08b has been approved by nikomatsakis

@bors 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 Oct 3, 2018
@bors
Copy link
Collaborator

bors commented Oct 5, 2018

⌛ Testing commit 8c6d08b with merge eaddf285fca351a00a98cb4996794ed2eebb46e3...

@bors
Copy link
Collaborator

bors commented Oct 5, 2018

💔 Test failed - status-travis

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

The job dist-armhf-linux 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. 
travis_time:end:0790a088:start=1538721235880881776,finish=1538721235888764958,duration=7883182
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1421a759
$ 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 --batch -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:00cd6410
travis_time:start:00cd6410
$ 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:030eb627
$ 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)

@davidtwco
Copy link
Member Author

@bors retry

@bors 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 Oct 5, 2018
@bors
Copy link
Collaborator

bors commented Oct 5, 2018

⌛ Testing commit 8c6d08b with merge 61f5ca7...

bors added a commit that referenced this pull request Oct 5, 2018
@bors
Auto merge of #54703 - davidtwco:issue-52086, r=nikomatsakis
61f5ca7 
error message when trying to move from an Rc or Arc is ungreat

Fixes #52086.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Oct 5, 2018

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

@bors bors merged commit 8c6d08b into rust-lang:master Oct 5, 2018
@bors bors mentioned this pull request Oct 5, 2018
Merged
@davidtwco davidtwco deleted the issue-52086 branch October 5, 2018 11:08
@pnkfelix pnkfelix added the A-NLL Area: Non-lexical lifetimes (NLL) label Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmandry tmandry tmandry left review comments

@nikomatsakis nikomatsakis nikomatsakis requested changes

Assignees

@nikomatsakis nikomatsakis

Labels
A-NLL Area: Non-lexical lifetimes (NLL) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@davidtwco @rust-highfive @tmandry @nikomatsakis @bors @pnkfelix