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.

Sign up for GitHub

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

Jump to bottom

Use our copy of RefCell for style data #11835

Merged
merged 2 commits into from
Jun 23, 2016
Merged

Use our copy of RefCell for style data #11835

merged 2 commits into from
Jun 23, 2016

Conversation

SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Jun 23, 2016 •
edited by larsbergstrom
Loading

This allows removing #![feature(as_unsafe_cell)] in geckolib and make progress towards #11815.

  • There are tests for these changes OR
  • These changes do not require new tests because they don’t introduce no functional changes.

This change is Reviewable

@SimonSapin
Use our copy of RefCell for style data.
f7f81e0 
This allows removing `#![feature(as_unsafe_cell)]` in geckolib
and make progress towards #11815.
@SimonSapin
Add commit hash to be reverted in FIXME comment.
6807bb6
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 23, 2016
@SimonSapin SimonSapin mentioned this pull request Jun 23, 2016
Closed
7 tasks
@nox
Copy link
Contributor

nox commented Jun 23, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 6807bb6 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 23, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit 6807bb6 with merge 3de14d9...

bors-servo pushed a commit that referenced this pull request Jun 23, 2016
Auto merge of #11835 - servo:diy-as-unsafe-cell, r=nox
3de14d9 
Use our copy of RefCell for style data

<!-- Please describe your changes on the following line: -->

This allows removing `#![feature(as_unsafe_cell)]` in geckolib and make progress towards #11815.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #11815.

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require new tests because they don’t introduce no functional changes.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11835)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo merged commit 6807bb6 into master Jun 23, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 23, 2016
@metajack
Copy link
Contributor

Is there justification for this somewhere? Ie, does the Rust team not want to stabilize this? Using custom data structures is a road that may end up infected crates that others depend on, and could segregate us from the ecosystem. Not to mention we won't be getting RefCell bugfixes.

@metajack metajack deleted the diy-as-unsafe-cell branch June 23, 2016 15:53
@SimonSapin
Copy link
Member Author

rust-lang/rust#27708 is the tracking issue for stabilizing as_unsafe_cell. Alex has expressed some concerns, but as far as I can tell it isn’t a firm "does not want to". I’m hoping this can be stabilized in the coming weeks, and once it rides the trains and reaches the stable channel we can revert this PR (except for the #![feature(as_unsafe_cell)] bit).

I agree that forking data structures from std is not great, but I’m not concerned about infecting more crates: nothing outside of Servo and Stylo depends on the style crate, and I don’t expect that to change.

I made this PR because I’d like to not be blocked several weeks (at best) before we can get CI for #11815.

@metajack
Copy link
Contributor

Thanks for the explanation.

@SimonSapin SimonSapin mentioned this pull request Jun 27, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SimonSapin @nox @bors-servo @metajack @highfive