Skip to content
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.

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

fixed the deprecated as_slice warning... #7741

Merged
merged 1 commit into from
Sep 25, 2015

Conversation

wafflespeanut
Copy link
Contributor

I've put its original implementation from core/option.rs instead of the dear departed as_slice

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 25, 2015
@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@wafflespeanut
Copy link
Contributor Author

Hmm, we could have something else...

@jdm
Copy link
Member

jdm commented Sep 25, 2015

Why not call slice::ref_slice instead of inlining it?

@metajack
Copy link
Contributor

Replacing safe code with unsafe code seems not great. This is making the code worse.

What other workarounds are available for the lack of as_slice?

@wafflespeanut
Copy link
Contributor Author

Yep, I should've gone for ref_slice.

},
None => {
let empty_arr: &[_] = &[];
empty_arr
Copy link
Member

Choose a reason for hiding this comment

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

Can these two lines be combined into a single &[] expression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course...

@jdm
Copy link
Member

jdm commented Sep 25, 2015

@bors-servo: r+
Lovely!

@bors-servo
Copy link
Contributor

📌 Commit 35d7ced has been approved by jdm

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Sep 25, 2015
@wafflespeanut
Copy link
Contributor Author

Thanks! :)

@bors-servo
Copy link
Contributor

⌛ Testing commit 35d7ced with merge b7c003f...

bors-servo pushed a commit that referenced this pull request Sep 25, 2015
fixed the deprecated `as_slice` warning...

I've put its original implementation from [`core/option.rs`](http://doc.servo.org/src/core/option.rs.html#692) instead of the dear departed `as_slice`

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7741)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit 35d7ced into servo:master Sep 25, 2015
@wafflespeanut wafflespeanut deleted the warning_fix branch September 25, 2015 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants