Skip to content

Update dependencies - #203

Merged
bors-servo merged 2 commits into
servo:masterfrom
hcpl:update-deps
Apr 10, 2018
Merged

Update dependencies#203
bors-servo merged 2 commits into
servo:masterfrom
hcpl:update-deps

Conversation

@hcpl

@hcpl hcpl commented Apr 1, 2018

Copy link
Copy Markdown
Contributor

This change is Reviewable

@SimonSapin

Copy link
Copy Markdown
Member

Let’s coordinate this with other Servo dependencies: servo/servo#20497

@SimonSapin

Copy link
Copy Markdown
Member

Looks good, please also remove AsciiExt imports in order to compile on Nightly and increment version numbers of both crates. Thanks!


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


string-cache-codegen/lib.rs, line 195 at r1 (raw file):

        let atoms_ref = &atoms;
        let empty_string_index = atoms.iter().position(|s| s.is_empty()).unwrap() as u32;
        let data = (0..atoms.len()).map(|i| proc_macro2::Literal::u64_suffixed(shared::pack_static(i as u32)));

It’s unfortunate that we can’t use hex literals anymore but oh well, it’s not that important.


Comments from Reviewable

@hcpl

hcpl commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

We can do a trick with quote! if we need it: https://play.rust-lang.org/?gist=33740da5f95db760cc3b515486f643af&version=stable.

@SimonSapin

Copy link
Copy Markdown
Member

quote! only works with a literal (statically-known) value. But maybe something like format!("0x{:016x}").parse::<TokenStream>().unwrap() would work?

@SimonSapin

Copy link
Copy Markdown
Member

That said we don’t really need it, try this if you feel like it.

Also fix an unnoticed bug and render data as hex numbers.
@hcpl

hcpl commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

But maybe something like format!("0x{:016x}").parse::<TokenStream>().unwrap() would work?

Ah yeah, this is the right one.

And I've implemented it anyway so that numbers in ${OUT_DIR}/test_atom.rs look better.

@SimonSapin

Copy link
Copy Markdown
Member

Thanks!

@bors-servo r+


Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@bors-servo

Copy link
Copy Markdown
Contributor

📌 Commit f392c9b has been approved by SimonSapin

@bors-servo

Copy link
Copy Markdown
Contributor

⌛ Testing commit f392c9b with merge 89e0ef8...

bors-servo pushed a commit that referenced this pull request Apr 10, 2018
Update dependencies

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

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-travis
Approved by: SimonSapin
Pushing 89e0ef8 to master...

@bors-servo
bors-servo merged commit f392c9b into servo:master Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants