Skip to content

Remove transmute in From<&str> impls for Arc/Rc #47004

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 1 commit into from
Dec 31, 2017
Merged

Remove transmute in From<&str> impls for Arc/Rc #47004

merged 1 commit into from
Dec 31, 2017

Conversation

nvzqz
Copy link
Contributor

@nvzqz nvzqz commented Dec 25, 2017

Performs conversion via raw pointer casts.

@rust-highfive
Copy link
Contributor

r? @bluss

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

@bluss
Copy link
Member

bluss commented Dec 25, 2017

I'd prefer transmute here over going through Arc::from_raw, as long as the pointer arithmetic in into/from doesn't optimize out and cancel out as it should. Last time I checked, it didn't.

@nvzqz
Copy link
Contributor Author

nvzqz commented Dec 25, 2017

I forgot about that aspect of into/from here. That's a good point. 🤔

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2017
@bluss
Copy link
Member

bluss commented Dec 28, 2017

When double checking in playground, it optimizes out, so then it's fine for me.

A reviewer can r=me but I wanted to figure out if travis will be green if we rerun it (after time out in scache?)

@nvzqz
Copy link
Contributor Author

nvzqz commented Dec 28, 2017

Thanks for checking the optimization out 😄

@kennytm
Copy link
Member

kennytm commented Dec 31, 2017

@bors r=bluss,kennytm

Can't get pass the spurious error without rebasing. The change looks reasonable, just let bors to do the merge step.

@bors
Copy link
Collaborator

bors commented Dec 31, 2017

📌 Commit 0fbcb7b has been approved by bluss,kennytm

@kennytm kennytm 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 Dec 31, 2017
@bors
Copy link
Collaborator

bors commented Dec 31, 2017

⌛ Testing commit 0fbcb7b with merge e037423...

bors added a commit that referenced this pull request Dec 31, 2017
@bors
Auto merge of #47004 - nvzqz:rc-conversions, r=bluss,kennytm
e037423 
Remove transmute in From<&str> impls for Arc/Rc

Performs conversion via raw pointer casts.
@bors
Copy link
Collaborator

bors commented Dec 31, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: bluss,kennytm
Pushing e037423 to master...

@bors bors merged commit 0fbcb7b into rust-lang:master Dec 31, 2017
@nvzqz nvzqz deleted the rc-conversions branch December 31, 2017 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bluss bluss

Labels
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.
5 participants
@nvzqz @rust-highfive @bluss @kennytm @bors