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 upcodegen: Fix dictionary handling and semantics #11155
Conversation
highfive
commented
May 12, 2016
|
Heads up! This PR modifies the following files:
|
highfive
commented
May 12, 2016
|
r? @asajeffrey Not sure about how to test it, since we only trigger mozbrowser errors on panics... I'll look for some other reserved keyword in the idls. |
|
Indeed, testing is going to be tricky. |
|
r=me once the testing story is worked out. |
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
|
I'm having local problems with the test I've just written, so I'm going to do a try run. The problem is that for some reason even when the So the only output I'm seeing is:
|
|
@bors-servo: try |
codegen: Use the non-mangled name in set_dictionary_property 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 #11152 (github issue number if applicable). Either: - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. Fixes #11152 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11155) <!-- Reviewable:end -->
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
|
@bors-servo: try- clean try (Fixed tidy) |
|
Rather than corrupting the stack, I suspect that a generated binding function is returning false at some point which silently suspends JS execution. |
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
1 similar comment
highfive
commented
May 12, 2016
|
New code was committed to pull request. |
|
This should be ready for re-review. The problem was that when we convert a rust dictionary to js, we setted the properties even when they were Going to do a try run to check if we rely on the old semantics: @bors-servo: try |
codegen: Fix dictionary handling and semantics 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 #11152 (github issue number if applicable). Either: - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. Fixes #11152 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11155) <!-- Reviewable:end -->
|
|
highfive
commented
May 13, 2016
|
highfive
commented
May 13, 2016
|
New code was committed to pull request. |
|
Updated test expectations, the cors test seems to be intermittent, the other failures are old expected timeouts uncovered by this patch. @bors-servo: r=Ms2ger |
|
|
codegen: Fix dictionary handling and semantics 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 #11152 (github issue number if applicable). Either: - [x] There are tests for these changes OR - [ ] These changes do not require tests because _____ Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. Fixes #11152 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11155) <!-- Reviewable:end -->
|
|
highfive
commented
May 13, 2016
|
|
@bors-servo: retry |
|
|
|
|
emilio commentedMay 12, 2016
•
edited
Thank you for contributing to Servo! Please replace each
[ ]by[X]when the step is complete, and replace__with appropriate data:./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsEither:
Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.
Fixes #11152
This change is