Run on Rust stable #95

SimonSapin · 2015-07-18T22:14:22Z

Without --features unstable:

Atom is 16 bytes instead of 8. (It has a drop flag.)
ns! and atom! are giant generated macros instead of plugins, and so may increase compile times.

r? @glennw, particularly c4fa256.

glennw · 2015-07-20T22:51:44Z

Reviewed 13 of 14 files at r1.
Review status: 13 of 14 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

^{.travis.yml, line 8 [r1] (raw file):}
Is this saying we don't run the tests on anything but nightly? Is there a reason for that?

^{src/atom/mod.rs, line 91 [r1] (raw file):}
I think this still handles the ABA case mentioned in the comments. Might be worth a double check though.

^{src/atom/mod.rs, line 234 [r1] (raw file):}
Do we still need this check for the optimized case?

Comments from the review on Reviewable.io

SimonSapin · 2015-07-20T23:12:16Z

Review status: 10 of 14 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

^{.travis.yml, line 8 [r1] (raw file):}
Yes, because benchmarks and some tests require unstable features. I’ve added a commit to disable just those when --feature unstable is not given.

^{src/atom/mod.rs, line 234 [r1] (raw file):}
I don’t understand. Need what in what case? There is no refcount to decrement if an atom is not dynamic.

Comments from the review on Reviewable.io

SimonSapin · 2015-07-21T15:28:13Z

Review status: 10 of 14 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

^{src/atom/mod.rs, line 91 [r1] (raw file):}
I believe the logic is the same as before in all three cases:

ptr is null: allocate a new entry and return a pointer to that entry.
ptr is not null, and the refcount was zero before we incremented it: decrement it again, allocate a new entry and return a pointer to that entry.
ptr is not null, and the refcount was positive: return ptr unchanged.

Comments from the review on Reviewable.io

bors-servo · 2015-07-24T19:16:44Z

☔ The latest upstream changes (presumably #96) made this pull request unmergeable. Please resolve the merge conflicts.


        Remove usage of unstable features in string_cache_shared


        Use phf_codegen instead of phf_macros


        Lower ENTRY_ALIGNMENT from 16 to 4

@gw

We only need 2 bits to store the tag (which is 0b_00, 0b_01, or 0b_10). This will enable using `Box::new` instead of `heap::allocate`. @gw, was there another reason to have it at 16?


        Update to filling drop


        Move remaining usage of unstable features behind a `unstable` Cargo f…

…eature flag. Without `--features unstable`: * `Atom` is 16 bytes instead of 8. (It has a drop flag.) * `ns!` and `atom!` are giant generated macros instead of plugins, and so may increase compile time.


        Have Travis run `cargo build` on Rust stable and beta


        Only make warnings fatal when testing.

That way, a deprecation warning won’t break dependants.


        Also test with unstable features on Travis.


        Make rand a dev-dependency. (It’s only used in tests.)


        Explicitly drop the result of box_from_raw()

It’s not necessary, but makes it clearer to readers what’s going on.


        Gratuitious refactoring.


        Have Travis run (some) tests on stable Rust.


        Fix #70 using Option<Box<StringCacheEntry>> instead of *mut StringCac…

…heEntry

glennw · 2015-07-26T21:13:49Z

Reviewed 1 of 14 files at r1.
Review status: 9 of 13 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

Comments from the review on Reviewable.io

glennw · 2015-07-26T21:14:21Z

@SimonSapin looks good to me.

SimonSapin · 2015-07-27T06:01:38Z

@bors-servo r=glennw

bors-servo · 2015-07-27T06:01:39Z

📌 Commit f7fd964 has been approved by glennw

bors-servo · 2015-07-27T06:01:40Z

⌛ Testing commit f7fd964 with merge dd1b626...


        Auto merge of #95 - servo:stable, r=glennw

@glennw

Run on Rust stable Without `--features unstable`: * `Atom` is 16 bytes instead of 8. (It has a drop flag.) * `ns!` and `atom!` are giant generated macros instead of plugins, and so may increase compile times. r? @glennw, particularly c4fa256. Needed for servo/html5ever#53  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/string-cache/95)

bors-servo · 2015-07-27T06:03:48Z

☀️ Test successful - travis


        Enable unstable feature in string-cache.

See servo/string-cache#95


        Enable unstable feature in string-cache.

See servo/string-cache#95

SimonSapin force-pushed the stable branch 3 times, most recently from 8cbd44a to 17ced83 Jul 18, 2015

SimonSapin added 12 commits Jul 18, 2015

Remove usage of unstable features in string_cache_shared

5169a5b

Use phf_codegen instead of phf_macros

917dd80

Lower ENTRY_ALIGNMENT from 16 to 4

747bf24

We only need 2 bits to store the tag (which is 0b_00, 0b_01, or 0b_10). This will enable using `Box::new` instead of `heap::allocate`. @gw, was there another reason to have it at 16?

Update to filling drop

36fb311

Move remaining usage of unstable features behind a `unstable` Cargo f…

8a3f35c

…eature flag. Without `--features unstable`: * `Atom` is 16 bytes instead of 8. (It has a drop flag.) * `ns!` and `atom!` are giant generated macros instead of plugins, and so may increase compile time.

Have Travis run `cargo build` on Rust stable and beta

d75004f

Only make warnings fatal when testing.

ba28f51

That way, a deprecation warning won’t break dependants.

Also test with unstable features on Travis.

4706e16

Make rand a dev-dependency. (It’s only used in tests.)

2d146b5

Explicitly drop the result of box_from_raw()

9de43cc

It’s not necessary, but makes it clearer to readers what’s going on.

Gratuitious refactoring.

7452b76

Have Travis run (some) tests on stable Rust.

Loading status checks…

66ad520

SimonSapin force-pushed the stable branch from 7765401 to 66ad520 Jul 24, 2015

SimonSapin mentioned this pull request Jul 24, 2015

Move out the list of static strings? #22

Closed

Fix #70 using Option<Box<StringCacheEntry>> instead of *mut StringCac…

Loading status checks…

f7fd964

…heEntry

SimonSapin deleted the stable branch Jul 27, 2015

SimonSapin added a commit to servo/servo that referenced this pull request Jul 30, 2015

Enable unstable feature in string-cache.

47b07ae

See servo/string-cache#95

josiahdaniels added a commit to josiahdaniels/servo that referenced this pull request Sep 28, 2015

Enable unstable feature in string-cache.

bc51328

See servo/string-cache#95

SimonSapin mentioned this pull request Nov 19, 2015

Remove the plugin? #124

Closed

servo / string-cache

Run on Rust stable #95

Run on Rust stable #95

SimonSapin commented Jul 18, 2015

glennw commented Jul 20, 2015

SimonSapin commented Jul 20, 2015

SimonSapin commented Jul 21, 2015

bors-servo commented Jul 24, 2015

glennw commented Jul 26, 2015

glennw commented Jul 26, 2015

SimonSapin commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 27, 2015

servo / string-cache

Join GitHub today

Run on Rust stable #95

Run on Rust stable #95

Conversation

SimonSapin commented Jul 18, 2015

glennw commented Jul 20, 2015

SimonSapin commented Jul 20, 2015

SimonSapin commented Jul 21, 2015

bors-servo commented Jul 24, 2015

glennw commented Jul 26, 2015

glennw commented Jul 26, 2015

SimonSapin commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 27, 2015

bors-servo commented Jul 27, 2015