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

feat(jsvalue) Add JsValue.ptr to retrieve internal idx value #2898

Closed
wants to merge 1 commit into from

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented May 17, 2022

I'm trying to solve #2231, and the first immediate step is to make JsValue.idx visible to the world. There is nothing particularly dangerous to allow that.

@alexcrichton
Copy link
Contributor

Personally I have always wanted to avoid this in wasm-bindgen if possible. This is a low-level implementation detail which is extremely hard to get right.

That being said it's been a long time since I originally felt that. I don't really mind that much any more, although I continue to feel that using this is highly likely to get yourself shot in the foot.

If this is added, though, then I think it should be done "officially". For example if there's a getter there should also be a constructor. There should ideally be more than a sentence of documentation about what the index is and how to use it. Additionally I think it would be good to mention this in the book about perhaps the design rationale behind the indices and how things work today.

@Hywan
Copy link
Contributor Author

Hywan commented May 19, 2022

I agree with everything. I'm not comfortable exposing idx but at some point it might be useful.

If this is added, though, then I think it should be done "officially". For example if there's a getter there should also be a constructor

This is dangerous to provide a constructor. The getter function should already be unsafe I guess now that I'm thinking about it. With a constructor, the user must guarantee that another JsValue with the same idx value does not exist.

I've a relatively stupid question: Is idx the ptr object property we see from JavaScript? If yes, it could be changed from JavaScript too, which is annoying now. Not sure how it syncs.

@alexcrichton
Copy link
Contributor

No the idx and ptr are not the same thing. The idx is an index into the private list of JS objects that the wasm holds a reference to, and the ptr is a pointer into linear memory for Rust-owned objects.

I don't think the getter needs to be unsafe, along the lines of as_raw_fd, but the constructor would be unsafe. This probably would also want an into_idx or similar as well.

@Hywan
Copy link
Contributor Author

Hywan commented May 11, 2023

Closing in favor of #3088.

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