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 upHandle u32 property indices #24858
Handle u32 property indices #24858
Conversation
highfive
commented
Nov 25, 2019
|
Heads up! This PR modifies the following files:
|
highfive
commented
Nov 25, 2019
|
|
|
@bors-servo try=wpt |
Handle u32 property indices <!-- Please describe your changes on the following line: --> Tried porting from Gecko, not sure how to do void JSID check yet. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [x] These changes fix #14093 <!-- Either: --> - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
| // } else { | ||
| // IdToInt32(cx, id); | ||
|
|
||
| // if !RUST_JSID_IS_VOID(raw_id) { |
This comment has been minimized.
This comment has been minimized.
jdm
Nov 25, 2019
Member
In the short term this could be if raw_id.asBits == JSID_VOID.asBits (based on https://doc.servo.org/mozjs_sys/jsid/constant.JSID_VOID.html). We can also add a RUST_JSID_IS_VOID API to https://github.com/servo/rust-mozjs/blob/master/src/glue.rs and https://github.com/servo/rust-mozjs/blob/master/src/jsglue.cpp.
| let mut i: u32 = 0; | ||
| let is_array = if s.is_ascii() { | ||
| let chars = s.as_bytes(); | ||
| StringIsArrayIndex1(chars.as_ptr() as *const i8, chars.len() as u32, &mut i) |
This comment has been minimized.
This comment has been minimized.
jdm
Nov 25, 2019
Member
c_char is a different type on ARM, so we need as *const _ instead of hardcoding i8.
Add JSID_IS_VOID For servo/servo#24858.
saschanaz
force-pushed the
saschanaz:handle-property
branch
3 times, most recently
from
b4b9a2a
to
ebaf585
|
@bors-servo r+ |
|
|
Handle u32 property indices <!-- Please describe your changes on the following line: --> Tried porting from Gecko, not sure how to do void JSID check yet. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14093 <!-- Either: --> - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
Ooops, last time important bit |
Handle u32 property indices <!-- Please describe your changes on the following line: --> Tried porting from Gecko, not sure how to do void JSID check yet. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14093 <!-- Either: --> - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
|
@bors-servo retry |
Handle u32 property indices <!-- Please describe your changes on the following line: --> Tried porting from Gecko, not sure how to do void JSID check yet. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14093 <!-- Either: --> - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
|
I will retry this when our CI has caught up with the existing build queue. Please do not retry in the meantime. |
|
Starting to worry a bit, but didn't find any evidence that this PR caused those failures. |
|
This PR is not the cause - we have several high frequency intermittent failures right now across all our CI platforms, and every time we have to do a full CI run it's very likely that we hit them. Retrying while there are builds in progress means we have to re-run all of the in-progress builds; whereas when we wait until the builds are complete, we only re-run the ones that failed. |
|
https://treeherder.allizom.org/#/jobs?repo=servo-auto shows the view that I am looking at to determine when it's a good time to retry. |
Sounds like a good reason to filter them, what's the reason not to do so? |
|
We filter based on test filename, and these are failures that can (and do) affect any test that runs. |
|
@bors-servo retry |
Handle u32 property indices <!-- Please describe your changes on the following line: --> Tried porting from Gecko, not sure how to do void JSID check yet. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #14093 <!-- Either: --> - [x] There are tests for these changes <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
saschanaz commentedNov 25, 2019
•
edited
Tried porting from Gecko, not sure how to do void JSID check yet.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors