Clean up some iterators #12861

nox · 2016-08-14T17:18:22Z

This change is


        Use impl DoubleEndedIterator for two flow_list iterators


        Simplify CSSStyleDeclaration::GetPropertyValue

While Fn(_) -> _ isn't Clone, fn(_) -> _ is.

highfive · 2016-08-14T17:18:25Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/cssstyledeclaration.rs

highfive · 2016-08-14T17:18:26Z

Warning

These commits modify layout and script code, but no tests are modified. Please consider adding a test!

nox · 2016-08-14T17:20:40Z

According to @eddyb, impl Trait isn't supposed to work when lifetimes are involved. Cc @nikomatsakis.

eddyb · 2016-08-14T17:35:41Z

It's supposed to (when the lifetimes are explicit) but I couldn't get anything to compile. Apparently the Item = &'a T projection bound forces the lifetime in the returned type to be exactly 'a.

notriddle · 2016-08-15T03:36:19Z

components/script/dom/cssstyledeclaration.rs

            // TODO: important is hardcoded to false because method does not implement it yet
-            let serialized_value = shorthand.serialize_shorthand_value_to_string(Map(list.iter()), false);
+            let serialized_value = shorthand.serialize_shorthand_value_to_string(
+                list.iter().map(Deref::deref as fn(_) -> _), false);


This use of a function pointer instead of the function's own anonymous type is to work around th fact that the latter isn't clone, right?

Yes, that's what I've written in the commit message.

Ms2ger · 2016-08-16T12:07:47Z

@bors-servo r+

bors-servo · 2016-08-16T12:07:49Z

📌 Commit 1637b0b has been approved by Ms2ger

bors-servo · 2016-08-16T12:15:23Z

⌛ Testing commit 1637b0b with merge d46bf6f...


        Auto merge of #12861 - nox:impl-trait, r=Ms2ger

Clean up some iterators  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12861)

bors-servo · 2016-08-16T13:02:44Z

💔 Test failed - linux-rel

highfive · 2016-08-16T13:02:47Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name.html:
  └ PASS [expected FAIL] Retaining window.name on history traversal

nox · 2016-08-16T13:33:31Z

@bors-servo retry #12568

bors-servo · 2016-08-16T13:49:09Z

⚡ Previous build results for arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev are reusable. Rebuilding only linux-rel...

bors-servo · 2016-08-16T14:37:23Z

💔 Test failed - linux-rel

highfive · 2016-08-16T14:37:25Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name.html:
  └ PASS [expected FAIL] Retaining window.name on history traversal

nox · 2016-08-16T16:20:22Z

@bors-servo retry #12568

bors-servo · 2016-08-16T17:02:03Z

⌛ Testing commit 1637b0b with merge bd446c5...


        Auto merge of #12861 - nox:impl-trait, r=Ms2ger

Clean up some iterators  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12861)

bors-servo · 2016-08-16T17:50:27Z

💔 Test failed - linux-rel

highfive · 2016-08-16T17:50:30Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name.html:
  └ PASS [expected FAIL] Retaining window.name on history traversal

nox · 2016-08-18T10:08:50Z

@bors-servo retry #12568

bors-servo · 2016-08-18T10:08:52Z

⌛ Testing commit 1637b0b with merge 4ca8370...


        Auto merge of #12861 - nox:impl-trait, r=Ms2ger

Clean up some iterators  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12861)

bors-servo · 2016-08-18T10:55:39Z

💔 Test failed - linux-rel

highfive · 2016-08-18T10:55:41Z

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/history-traversal/browsing_context_name.html:
  └ PASS [expected FAIL] Retaining window.name on history traversal

nox · 2016-08-18T10:56:48Z

Suddenly it doesn't look like an intermittent anymore, but how could my changes do that..?

notriddle · 2016-08-18T15:39:16Z

Change the expectation to PASS?

nox · 2016-08-20T18:34:14Z

@bors-servo r=Ms2ger

The test was disabled.

bors-servo · 2016-08-20T18:34:15Z

💡 This pull request was already approved, no need to approve it again.

This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
There's another pull request that is currently being tested, blocking this pull request: #12941

bors-servo · 2016-08-20T18:34:15Z

📌 Commit 1637b0b has been approved by Ms2ger

nox · 2016-08-20T18:35:02Z

Ugh.

@bors-servo retry

bors-servo · 2016-08-20T18:35:05Z

⌛ Testing commit 1637b0b with merge 8a75810...


        Auto merge of #12861 - nox:impl-trait, r=Ms2ger

Clean up some iterators  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12861)

bors-servo · 2016-08-20T20:07:25Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

nox added 2 commits Aug 14, 2016

Use impl DoubleEndedIterator for two flow_list iterators

a1af2cd

Simplify CSSStyleDeclaration::GetPropertyValue

Loading status checks…

1637b0b

While Fn(_) -> _ isn't Clone, fn(_) -> _ is.

highfive assigned jdm Aug 14, 2016

highfive added the S-awaiting-review label Aug 14, 2016

notriddle reviewed Aug 15, 2016
View changes

highfive assigned Ms2ger and unassigned jdm Aug 16, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Aug 16, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Aug 16, 2016

highfive added the S-tests-failed label Aug 16, 2016

highfive removed the S-tests-failed label Aug 16, 2016

highfive added the S-awaiting-merge label Aug 16, 2016

highfive removed the S-awaiting-merge label Aug 16, 2016

highfive added the S-tests-failed label Aug 16, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Aug 18, 2016

highfive removed the S-awaiting-merge label Aug 18, 2016

highfive added the S-tests-failed label Aug 18, 2016

highfive added S-awaiting-merge and removed S-tests-failed labels Aug 20, 2016

bors-servo merged commit 1637b0b into servo:master Aug 20, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Aug 20, 2016

bors-servo mentioned this pull request Aug 20, 2016

Merge normal and important declarations in style rules #12943

Merged

4 of 5 tasks complete

servo / servo

Clean up some iterators #12861

Clean up some iterators #12861

nox commented Aug 14, 2016 •

edited by larsbergstrom

highfive commented Aug 14, 2016

highfive commented Aug 14, 2016

nox commented Aug 14, 2016

eddyb commented Aug 14, 2016

This comment has been minimized.

This comment has been minimized.

Ms2ger commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 18, 2016

bors-servo commented Aug 18, 2016

bors-servo commented Aug 18, 2016

highfive commented Aug 18, 2016

nox commented Aug 18, 2016

notriddle commented Aug 18, 2016

nox commented Aug 20, 2016

bors-servo commented Aug 20, 2016

bors-servo commented Aug 20, 2016

nox commented Aug 20, 2016

bors-servo commented Aug 20, 2016

bors-servo commented Aug 20, 2016

servo / servo

Join GitHub today

Clean up some iterators #12861

Clean up some iterators #12861

Conversation

nox commented Aug 14, 2016 • edited by larsbergstrom

highfive commented Aug 14, 2016

highfive commented Aug 14, 2016

nox commented Aug 14, 2016

eddyb commented Aug 14, 2016

This comment has been minimized.

notriddle Aug 15, 2016

This comment has been minimized.

nox Aug 15, 2016

Ms2ger commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 16, 2016

bors-servo commented Aug 16, 2016

bors-servo commented Aug 16, 2016

highfive commented Aug 16, 2016

nox commented Aug 18, 2016

bors-servo commented Aug 18, 2016

bors-servo commented Aug 18, 2016

highfive commented Aug 18, 2016

nox commented Aug 18, 2016

notriddle commented Aug 18, 2016

nox commented Aug 20, 2016

bors-servo commented Aug 20, 2016

bors-servo commented Aug 20, 2016

nox commented Aug 20, 2016

bors-servo commented Aug 20, 2016

bors-servo commented Aug 20, 2016

nox commented Aug 14, 2016 •

edited by larsbergstrom