Skip to content

WebIDL Binder const char* return type handled correctly #15651

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CDLlo
Copy link

@CDLlo CDLlo commented Nov 29, 2021

Making a separate PR for the changes in the prior PR by ArthurG0. Unfortunately the branch being pulled from is not available for me to update/rebase and ArthurG0 left our company so it is not expected of them to follow up on the necessary changes for the stale PR to be accepted. I added them to the AUTHORS file in this branch to keep record of their contributions.

To restate the original PR, the original issue was that the WebIDL Binder was incorrectly handling the const char* return type as it treated it as an int rather than a char*. The fix allows the basic type to be cast into the correct type in the generated glue code.

@CDLlo CDLlo changed the title Copy of issue/14745 code changes WebIDL char* return type handled correctly Nov 29, 2021
@CDLlo CDLlo changed the title WebIDL char* return type handled correctly WebIDL Binder char* return type handled correctly Nov 29, 2021
@CDLlo CDLlo changed the title WebIDL Binder char* return type handled correctly WebIDL Binder const char* return type handled correctly Nov 29, 2021
@kripken
Copy link
Member

kripken commented Dec 2, 2021

This seems to also suffer from a CircleCI bug with not getting CI runs started. Sorry for this annoyance, I'm not sure what's wrong on their side. Hopefully they will respond soon to the ticket I opened for #15023

@kripken
Copy link
Member

kripken commented Jan 18, 2022

I am told by CircleCI that this bug with builds not starting may be worked around if you have a CircleCI account. If you do, then clicking the "Refresh Permissions" button on the "User Settings" page should do it. (If you do not have an account, then I'm not sure what's wrong, and I'll need to follow up with them.)

@kripken
Copy link
Member

kripken commented Jan 18, 2022

Actually builds seem to be running now after I resolved the merge conflict...

@kripken
Copy link
Member

kripken commented Jan 18, 2022

test_webidl_all_growth fails, which needs to be looked into.

Making a separate branch for an up to date PR. Also added original author to AUTHORS file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants