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

Move away from Root::deref. #4526

Merged
merged 15 commits into from Jan 2, 2015
Merged

Move away from Root::deref. #4526

merged 15 commits into from Jan 2, 2015

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 1, 2015

This is a start towards fixing #3868. Not all callers have been fixed yet, so the Deref implementation remains for now.

Ms2ger added 14 commits Jan 1, 2015
It does not add any safety, as the reference is constructed from a raw pointer
without limiting the lifetime in any way.
It does not add any safety, as the reference is constructed from a raw pointer
without limiting the lifetime in any way.
As it will be used much more widely after the upcoming changes, this limits
the effort reading and writing the method calls.
This changes those calls that were already sound.
This changes those calls whose unsoundness was not picked up by the type system
because of a lifetime constraint that cannot be expressed at this time.
Note that Root::clone() calls through to JSRef::clone() due to autoderef.
@highfive
Copy link

highfive commented Jan 1, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jan 1, 2015

Critic review: https://critic.hoppipolla.co.uk/r/3624

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented on 203d166 Jan 2, 2015

r+

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

saw approval from Manishearth
at 203d166

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

merging servo/servo/deref-1 = 203d166 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

servo/servo/deref-1 = 203d166 merged ok, testing candidate = 141b5d0

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

fast-forwarding master to auto = 141b5d0

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented on 203d166 Jan 2, 2015

r+

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

saw approval from Manishearth
at 203d166

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

merging servo/servo/deref-1 = 203d166 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

servo/servo/deref-1 = 203d166 merged ok, testing candidate = 141b5d0

This comment has been minimized.

Copy link
Contributor

bors-servo replied Jan 2, 2015

fast-forwarding master to auto = 141b5d0

bors-servo pushed a commit that referenced this pull request Jan 2, 2015
This is a start towards fixing #3868. Not all callers have been fixed yet, so the `Deref` implementation remains for now.
@bors-servo bors-servo closed this Jan 2, 2015
@bors-servo bors-servo merged commit 203d166 into master Jan 2, 2015
1 check passed
1 check passed
default all tests passed
@tetsuharuohzeki tetsuharuohzeki deleted the deref-1 branch Jan 2, 2015
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.