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

Update syn, quote and synstructure #20022

Merged
merged 5 commits into from Feb 13, 2018
Merged

Update syn, quote and synstructure #20022

merged 5 commits into from Feb 13, 2018

Conversation

@Eijebong
Copy link
Member

Eijebong commented Feb 11, 2018

Right now they're duplicated because we need a new serde release (ping @dtolnay), a new cssparser release (ping @SimonSapin) and a new release of html5ever with servo/html5ever#336


This change is Reviewable

@highfive
Copy link

highfive commented Feb 11, 2018

Heads up! This PR modifies the following files:

@nox
Copy link
Member

nox commented Feb 12, 2018

Fourth commit says it's about malloc_size_of_derive, but it also updates style_derive. Could you split the two and merge the "make tidy happy" commit be squashed with the commit that introduced the tidy failures?

Copy link
Member

nox left a comment

I reviewed the three first commits, will review the two last later.

@@ -2,27 +2,29 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#![recursion_limit = "128"]

This comment has been minimized.

@nox

nox Feb 12, 2018

Member

Why is this now needed?

This comment has been minimized.

@Eijebong

Eijebong Feb 12, 2018

Author Member

Because quote! recurses heavily and it failed to compile without this

This comment has been minimized.

@Eijebong

Eijebong Feb 12, 2018

Author Member

I'll try removing it to see if it was during a temporary state

This comment has been minimized.

@Eijebong

Eijebong Feb 12, 2018

Author Member

Doesn't compile without it, if that's a problem I guess I could split the quote! invocation in 2...

error: recursion limit reached while expanding the macro `stringify`
  --> components/domobject_derive/lib.rs:36:21
   |
36 |       let mut items = quote! {
   |  _____________________^
37 | |         impl #impl_generics ::js::conversions::ToJSValConvertible for #name #ty_generics #where_clause {
38 | |             #[allow(unsafe_code)]
39 | |             unsafe fn to_jsval(&self,
...  |
58 | |         }
59 | |     };
   | |_____^
syn = "0.11"
quote = "0.3.15"
syn = "0.12"
quote = "0.4"

This comment has been minimized.

@nox

nox Feb 12, 2018

Member

You sure about these versions, and that you don't require anything introduced in a minor release of either? Note that Gecko will use exactly 0.12 and 0.4 and thus it could fail to compile there. Just to be safe I would recommend setting the lowest bounds to 0.12.12 and 0.4.2.

This comment has been minimized.

@Eijebong

Eijebong Feb 12, 2018

Author Member

👍

@Eijebong Eijebong force-pushed the Eijebong:syn branch from de507f3 to 3c6f058 Feb 12, 2018
@Eijebong Eijebong force-pushed the Eijebong:syn branch from 3c6f058 to 9faa579 Feb 12, 2018
@nox
Copy link
Member

nox commented Feb 13, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2018

📌 Commit 9faa579 has been approved by nox

@highfive highfive assigned nox and unassigned Manishearth Feb 13, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2018

Testing commit 9faa579 with merge e488965...

bors-servo added a commit that referenced this pull request Feb 13, 2018
Update syn, quote and synstructure

Right now they're duplicated because we need a new serde release (ping @dtolnay), a new cssparser release (ping @SimonSapin) and a new release of html5ever with servo/html5ever#336

<!-- 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/20022)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2018

💔 Test failed - linux-rel-wpt

@emilio
Copy link
Member

emilio commented Feb 13, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2018

Testing commit 9faa579 with merge 6cb7805...

bors-servo added a commit that referenced this pull request Feb 13, 2018
Update syn, quote and synstructure

Right now they're duplicated because we need a new serde release (ping @dtolnay), a new cssparser release (ping @SimonSapin) and a new release of html5ever with servo/html5ever#336

<!-- 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/20022)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 13, 2018

@bors-servo bors-servo merged commit 9faa579 into servo:master Feb 13, 2018
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
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

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