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 upAssert that DOM structs have the correct first field #21105
Conversation
highfive
commented
Jun 29, 2018
|
Heads up! This PR modifies the following files:
|
highfive
commented
Jun 29, 2018
|
|
|
Huh, I thought that we already perform this check... Perhaps it isn't robust enough and it only checks whether the first field is also a DOM struct? |
|
Please describe what the changes do in the commit message, and maybe show what is generated here. I don't like how there is a special case for some canvas-related interface in the middle of codegen with this PR. |
|
|
||
| return """\ | ||
| impl %(selfName)s { | ||
| fn __assert_parent_type(&self) { |
This comment has been minimized.
This comment has been minimized.
nox
Jul 3, 2018
Member
I would rather encode the relation with a trait specifying from which interface it is extended.
This comment has been minimized.
This comment has been minimized.
Manishearth
Jul 3, 2018
Author
Member
You still need to assert the existence of this impl somehow, though. Thoughts?
|
Also please don't write |
|
Regarding canvas, I discussed this with @asajeffrey , who had written that, and for efficiency the paint rendering context struct internally "inherits" from the canvas rendering context, despite not webidl inheriting. This is still safe because canvas eventually inherits from EventTarget, so the layouts work out. Regarding |
You can use
Oh right. |
Manishearth
force-pushed the
Manishearth:dom-inheritance-assert
branch
from
b5e847a
to
0a8824a
DOM structs embed their parent type as their first field. This
introduces a `.parent()` method to the DOM struct that returns its first
field, and codegens a type assert that ensures that `.parent()` returns
the parent struct.
This generates:
On `#[dom_struct]`:
```rust
impl HasParent for Type {
type Parent = ParentType;
fn as_parent(&self) -> ParentType {
&self.first_field
}
}
```
In the codegen files:
```rust
impl Type {
fn __assert_parent_type(&self) {
let _: &ParentType = self.as_parent();
}
}
````
Manishearth
force-pushed the
Manishearth:dom-inheritance-assert
branch
from
0a8824a
to
ad19899
|
Created a HasParent trait and fixed the commit message. Left a FIXME for the @bors r=nox |
|
@bors-servo r=nox |
|
|
Assert that DOM structs have the correct first field Not having the right field can lead to fun bugs like ferjm#1 where the struct gets mis-reinterpreted as what should be its parent (but is not layout-wise) <!-- 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/21105) <!-- Reviewable:end -->
|
|
Manishearth commentedJun 29, 2018
•
edited by nox
Not having the right field can lead to fun bugs like ferjm#1 where the struct gets mis-reinterpreted as what should be its parent (but is not layout-wise)
This change is