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

Create a second set of borrowing wrappers #422

Merged
merged 1 commit into from Apr 17, 2018
Merged

Conversation

@marmistrz
Copy link
Contributor

marmistrz commented Apr 11, 2018

@nox
Copy link
Member

nox commented Apr 11, 2018

Can you describe the change in the commit message, please?

Our current wrappers held in rust::wrappers accept the Handles by
consume/Copy. This was introduced in #393 to make it feasible to migrate
the API calls on the Servo side in finite time. This, in particular,
requires unsafe code in the implementation of MutableHandle.

This change introduces a second set of wrappers, which will now borrow
MutableHandles instead of relying on impl Copy for MutableHandle. The
Servo code should gradually move to the new wrappers,
rust::jsapi_wrapped, so that we can eventually remove rust::wrappers.
@marmistrz marmistrz force-pushed the marmistrz:wrappers2 branch from 3edba9e to 6ca0c13 Apr 11, 2018
@marmistrz
Copy link
Contributor Author

marmistrz commented Apr 11, 2018

@nox done

@nox
Copy link
Member

nox commented Apr 17, 2018

@bors-servo r+

Thanks for your work and patience, sorry for taking so long to review this.

@bors-servo
Copy link
Contributor

bors-servo commented Apr 17, 2018

📌 Commit 6ca0c13 has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Apr 17, 2018

Testing commit 6ca0c13 with merge 89fc3e8...

bors-servo added a commit that referenced this pull request Apr 17, 2018
Create a second set of borrowing wrappers

As agreed here:
https://mozilla.logbot.info/servo/20180326#c14521098-c14521118
https://mozilla.logbot.info/servo/20180405#c14564933

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

bors-servo commented Apr 17, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nox
Pushing 89fc3e8 to master...

@bors-servo bors-servo merged commit 6ca0c13 into servo:master Apr 17, 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

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