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

Remove some usage of unstable features #18839

Merged
merged 7 commits into from Oct 12, 2017
Merged

Remove some usage of unstable features #18839

merged 7 commits into from Oct 12, 2017

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Oct 11, 2017

Box::new(x) seems to compile to exactly the same assembly as box x in release mode:
https://play.rust-lang.org/?gist=55ec5dd02c22fa799375d6e2e1a6e970&version=nightly


This change is Reviewable

@highfive
Copy link

highfive commented Oct 11, 2017

Heads up! This PR modifies the following files:

  • @asajeffrey: components/constellation/network_listener.rs, components/constellation/lib.rs, components/constellation/pipeline.rs
  • @KiChjang: components/net/lib.rs, components/script_plugins/lib.rs, components/script/lib.rs, components/net_traits/lib.rs, components/net_traits/lib.rs and 7 more
  • @paulrouget: ports/glutin/lib.rs, components/compositing/lib.rs, components/constellation/network_listener.rs, components/constellation/lib.rs, components/constellation/pipeline.rs and 1 more
  • @fitzgen: components/script_plugins/lib.rs, components/devtools/actors/framerate.rs, components/devtools/actors/object.rs, components/script/lib.rs, components/profile/lib.rs and 11 more
  • @emilio: components/layout/display_list_builder.rs, components/layout/text.rs, components/layout/block.rs, components/layout/layout_debug.rs, components/layout/construct.rs and 4 more
@highfive
Copy link

highfive commented Oct 11, 2017

warning Warning warning

  • These commits modify net, layout, gfx, and script code, but no tests are modified. Please consider adding a test!
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 12, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2017

Trying commit 7cc3f4f with merge 037c683...

bors-servo added a commit that referenced this pull request Oct 12, 2017
Remove some usage of unstable features

`Box::new(x)` seems to compile to exactly the same assembly as `box x` in release mode:
https://play.rust-lang.org/?gist=55ec5dd02c22fa799375d6e2e1a6e970&version=nightly

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18839)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2017

@nox
Copy link
Member

nox commented Oct 12, 2017

IIRC removing box caused perf regressions on Dromaeo in the past.

@nox
nox approved these changes Oct 12, 2017
Copy link
Member

nox left a comment

3 remarks, but r=me if you don't want to make the suggested changes.

@@ -109,7 +109,8 @@ fn is_image_opaque(format: webrender_api::ImageFormat, bytes: &[u8]) -> bool {
fn premultiply(data: &mut [u8]) {
let length = data.len();

for i in Iterator::step_by(0..length, 4) {
let mut i = 0;
while i < length {

This comment has been minimized.

Copy link
@nox

nox Oct 12, 2017

Member

What about using <[_]>::chunks?

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Oct 12, 2017

Author Member

The last item of chunks can be a smaller slice, whereas this loop will stop before.

@@ -50,14 +50,17 @@ pub struct ImageMetadata {
fn byte_swap_and_premultiply(data: &mut [u8]) {
let length = data.len();

for i in Iterator::step_by(0..length, 4) {
let mut i = 0;
while i < length {

This comment has been minimized.

Copy link
@nox

nox Oct 12, 2017

Member

What about using <[_]>::chunks?


fn try_from(position: &'a str) -> Result<AdjacentPosition, Self::Error> {
impl AdjacentPosition {
fn try_from(position: &str) -> Result<AdjacentPosition, Error> {

This comment has been minimized.

Copy link
@nox

nox Oct 12, 2017

Member

Could you use FromStr here?

@SimonSapin SimonSapin force-pushed the stabler branch from 7cc3f4f to 8ee51bd Oct 12, 2017
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 12, 2017

@bors-servo r=nox

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2017

📌 Commit 8ee51bd has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2017

Testing commit 8ee51bd with merge fc4de5a...

bors-servo added a commit that referenced this pull request Oct 12, 2017
Remove some usage of unstable features

`Box::new(x)` seems to compile to exactly the same assembly as `box x` in release mode:
https://play.rust-lang.org/?gist=55ec5dd02c22fa799375d6e2e1a6e970&version=nightly

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18839)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 12, 2017

@bors-servo bors-servo merged commit 8ee51bd into master Oct 12, 2017
3 of 4 checks passed
3 of 4 checks passed
dependency-ci Failed dependency checks
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.