Skip to content
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

Example of a custom trace hook #82

Closed
wants to merge 0 commits into from
Closed

Example of a custom trace hook #82

wants to merge 0 commits into from

Conversation

hotsphink
Copy link
Collaborator

I was lazy, and updated to -std=c++20 to use a designated initializer. It makes creating JSClassOps much quicker and nicer.

@hotsphink
Copy link
Collaborator Author

Er, I messed up and based this on my local custom.err branch. Which has now landed. I rebased it onto the landed esr115 tip and re-pushed. I don't know if I need to do anything else.

@ptomato
Copy link
Collaborator

ptomato commented Jul 10, 2024

No problem. If I was reviewing it I'd have one question about whether to use JS::GetMaybePtrFromReservedSlot() in CustomObject::ownedBox() and CustomObject::unownedBox(). On the one hand it seems like we don't need the isUndefined() check, but on the other hand we do check that the pointer isn't null before tracing.

@hotsphink
Copy link
Collaborator Author

I tried using JS::GetMaybePtrFromReservedSlot first, and got a compile error. But I just tried it again, and it worked fine. I must have done something wrong. (I think I misinterpreted the error to be saying that it needed my object type to inherit from JSObject, which isn't possible, but that's no different from any other API that takes JSObject*.)

I guess I'll make a PR for the change, and you can add any other comments there.

@hotsphink
Copy link
Collaborator Author

#84

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants