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

Fix the build for NLL #21356

Merged
merged 2 commits into from Aug 24, 2018

Conversation

Projects
None yet
9 participants
@SimonSapin
Copy link
Member

SimonSapin commented Aug 7, 2018

Test with RUSTFLAGS="-Zborrowck=mir -Ztwo-phase-borrows" cargo build

https://internals.rust-lang.org/t/help-us-get-non-lexical-lifetimes-nll-over-the-finish-line/7807/7


This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Aug 7, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webglrenderingcontext.rs, components/script/dom/imagedata.rs, components/script/dom/window.rs
  • @KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/imagedata.rs, components/script/dom/window.rs
  • @emilio: components/layout/table.rs
@highfive

This comment has been minimized.

Copy link

highfive commented Aug 7, 2018

warning Warning warning

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

emilio approved these changes Aug 7, 2018

Copy link
Member

emilio left a comment

Looks good :)

self.column_computed_inline_sizes[index].size +=
remaining_inline_size.scale_by(
self.column_computed_inline_sizes[index].size.0 as f32 / total_minimum_size);

This comment has been minimized.

@emilio

emilio Aug 7, 2018

Member

This looks a bit like a regression doesn't it?

This comment has been minimized.

@KiChjang

KiChjang Aug 8, 2018

Member

This is possibly rust-lang/rust#47349, which I'm working on.

This comment has been minimized.

@SimonSapin

SimonSapin Aug 8, 2018

Author Member

Per rust-lang/rust#47349 (comment) the new borrowck behavior is considered correct. That issue is only still open in order to improve the error message.

@@ -531,7 +531,7 @@ impl Flow for TableFlow {
self.block_flow.build_display_list_for_block(state, border_painting_mode);

let iter = TableCellStyleIterator::new(&self);
for mut style in iter {
for style in iter {

This comment has been minimized.

@emilio

emilio Aug 7, 2018

Member

Just curious, are these just to avoid warnings?

This comment has been minimized.

@SimonSapin

SimonSapin Aug 8, 2018

Author Member

Yes, I suppose that they were false negatives for the warning under AST-borrowck.

@SimonSapin

This comment has been minimized.

Copy link
Member Author

SimonSapin commented Aug 8, 2018

Oops I accidentally pushed this to master :/ Thanks @jdm for taking care of reverting. Don’t we have github settings to prevent this from happening, though?

@SimonSapin

This comment has been minimized.

Copy link
Member Author

SimonSapin commented Aug 8, 2018

Per #21358 it looks like unrelated changes in url fixed some tests, so this PR will need to update expectations.

@bors-servo try

bors-servo added a commit that referenced this pull request Aug 8, 2018

Auto merge of #21356 - servo:nll, r=<try>
Fix the build for NLL

Test with `RUSTFLAGS="-Zborrowck=mir -Ztwo-phase-borrows" cargo build`

https://internals.rust-lang.org/t/help-us-get-non-lexical-lifetimes-nll-over-the-finish-line/7807/7

<!-- 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/21356)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 8, 2018

⌛️ Trying commit e510dea with merge 347d90c...

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 8, 2018

💔 Test failed - linux-rel-wpt

@SimonSapin

This comment has been minimized.

Copy link
Member Author

SimonSapin commented Aug 8, 2018

This should be landable now. r? @emilio (or anyone)

@highfive highfive assigned emilio and unassigned paulrouget Aug 8, 2018

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Aug 8, 2018

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 8, 2018

📌 Commit ff77763 has been approved by jdm

@highfive highfive assigned jdm and unassigned emilio Aug 8, 2018

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Aug 8, 2018

@bors-servo retry

@jdm jdm closed this Aug 8, 2018

@jdm jdm reopened this Aug 8, 2018

@jdm

This comment has been minimized.

Copy link
Member

jdm commented Aug 8, 2018

@bors-servo force r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

📌 Commit 5d40886 has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

⌛️ Testing commit 5d40886 with merge dcc8a6e...

bors-servo added a commit that referenced this pull request Aug 24, 2018

Auto merge of #21356 - servo:nll, r=jdm
Fix the build for NLL

Test with `RUSTFLAGS="-Zborrowck=mir -Ztwo-phase-borrows" cargo build`

https://internals.rust-lang.org/t/help-us-get-non-lexical-lifetimes-nll-over-the-finish-line/7807/7

<!-- 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/21356)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

💔 Test failed - linux-dev

@SimonSapin

This comment has been minimized.

Copy link
Member Author

SimonSapin commented Aug 24, 2018

WPT manifest updated.

@bors-servo r=jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

📌 Commit 1333b6e has been approved by jdm

@servo-wpt-sync

This comment has been minimized.

Copy link
Collaborator

servo-wpt-sync commented Aug 24, 2018

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at jdm/web-platform-tests#78.

bors-servo added a commit that referenced this pull request Aug 24, 2018

Auto merge of #21356 - servo:nll, r=jdm
Fix the build for NLL

Test with `RUSTFLAGS="-Zborrowck=mir -Ztwo-phase-borrows" cargo build`

https://internals.rust-lang.org/t/help-us-get-non-lexical-lifetimes-nll-over-the-finish-line/7807/7

<!-- 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/21356)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

⌛️ Testing commit 1333b6e with merge 3955119...

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Aug 24, 2018

@bors-servo bors-servo referenced this pull request Aug 24, 2018

Merged

Rustfmt layout crate #21509

@bors-servo bors-servo merged commit 1333b6e into master Aug 24, 2018

3 of 4 checks passed

Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@servo-wpt-sync

This comment has been minimized.

Copy link
Collaborator

servo-wpt-sync commented Aug 24, 2018

Error syncing changes upstream. Logs saved in error-snapshot-1535130643349.

@SimonSapin SimonSapin deleted the nll branch Aug 24, 2018

jdm added a commit to web-platform-tests/wpt that referenced this pull request Aug 24, 2018

@servo-wpt-sync servo-wpt-sync referenced this pull request Aug 24, 2018

Merged

Update getopts #12681

@SimonSapin SimonSapin referenced this pull request Aug 26, 2018

Merged

Optional argument #53

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Aug 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.