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

Mark TypedRect as repr(C) #216

Merged
merged 1 commit into from Jun 21, 2017
Merged

Mark TypedRect as repr(C) #216

merged 1 commit into from Jun 21, 2017

Conversation

@eqrion
Copy link
Contributor

eqrion commented Jun 21, 2017

TypedPoint2D and TypedSize2D are both repr(C) so TypedRect can be marked repr(C). This helps improve the FFI interface for WebRender.


This change is Reviewable

@emilio
Copy link
Member

emilio commented Jun 21, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2017

📌 Commit 6c00093 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2017

Testing commit 6c00093 with merge 1d5fb01...

bors-servo added a commit that referenced this pull request Jun 21, 2017
Mark TypedRect as repr(C)

TypedPoint2D and TypedSize2D are both repr(C) so TypedRect can be marked repr(C). This helps improve the FFI interface for WebRender.

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

emilio commented Jun 21, 2017

Thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Jun 21, 2017

☀️ Test successful - status-travis
Approved by: emilio
Pushing 1d5fb01 to master...

@bors-servo bors-servo merged commit 6c00093 into servo:master Jun 21, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@eqrion eqrion deleted the eqrion:rect/repr-c branch Jun 21, 2017
@kvark
Copy link
Member

kvark commented Jun 21, 2017

@nical I hate to say it, but isn't adding repr() technically making this a breaking change?..

@nical
Copy link
Collaborator

nical commented Jun 22, 2017

I don't think so. As long as there is no repr specified the representation isn't defined, so code that relies on the representation is already technically invalid. It could have been a breaking change if the type already had an explicit repr and we had changed it.
Also, in practice the actual representation has not changed in that case anyway, so it isn't possible that this has broken any code as far as I know.

@eqrion
Copy link
Contributor Author

eqrion commented Jun 22, 2017

@nical It would be nice to get a version with this used by webrender, so it can make its way over to gecko eventually.

Not sure how much work that is, it's not urgent.

@nical
Copy link
Collaborator

nical commented Jun 22, 2017

I think that the only thing we need to do is bump euclid to 0.15.1 and have webrender depend on 0.15.1 instead of 0.15.0 (since there was no breaking change as far as I can tell).

@eqrion eqrion mentioned this pull request Jun 22, 2017
bors-servo added a commit that referenced this pull request Jun 22, 2017
Bump version to 0.15.1

This is for getting #216 into webrender.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/218)
<!-- Reviewable:end -->
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

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