Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upCall BoxTreeRoot::construct from layout_thread_2020 #24360
Conversation
highfive
commented
Oct 3, 2019
|
|
| unsafe impl Send for ServoLayoutNode<'_> {} | ||
| unsafe impl Sync for ServoLayoutNode<'_> {} |
This comment has been minimized.
This comment has been minimized.
SimonSapin
Oct 3, 2019
Member
I there a comment we could add here about the soundness of these impls?
This comment has been minimized.
This comment has been minimized.
| @@ -17,7 +17,7 @@ use style::properties::ComputedValues; | |||
| use style::selector_parser::PseudoElement; | |||
|
|
|||
| #[derive(Clone, Copy)] | |||
| pub(super) enum WhichPseudoElement { | |||
| pub enum WhichPseudoElement { | |||
This comment has been minimized.
This comment has been minimized.
SimonSapin
Oct 3, 2019
•
Member
Are all of these newly-pub items used in another crate? I wonder if could move the crate boundaries somehow to make the API surface smaller. Maybe add some wrapper structs with private fields, for things that are exposed?
This comment has been minimized.
This comment has been minimized.
nox
Oct 4, 2019
•
Author
Member
The trait NodeExt needs to be public, and thus any item exposed by the trait needs to be public too, so WhichPseudoElement, among other types, needs to be public. I could wrap LayoutBox into a struct but I'm not sure that it is that useful to do so.
|
@bors-servo r+ |
|
|
Call BoxTreeRoot::construct from layout_thread_2020 <!-- Reviewable:start --> This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24360) <!-- Reviewable:end -->
|
|
nox commentedOct 3, 2019
•
edited by SimonSapin
This change is