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

__wbindgen_thread_destroy has optional params #3703

Merged

Conversation

AlexErrant
Copy link
Contributor

@AlexErrant AlexErrant commented Nov 12, 2023

This PR updates the generated TypeScript to reflect the fact that __wbindgen_thread_destroy has optional params.

Ref:

// if no explicit parameters are passed (i.e. their value is 0) then we assume
// we're being called from the agent that must be destroyed and rely on its globals

Before:

export interface InitOutput {
  readonly start: () => void;
  readonly run: () => void;
  readonly child_entry_point: (a: number) => void;
  readonly memory: WebAssembly.Memory;
  readonly __wbindgen_malloc: (a: number, b: number) => number;
  readonly __wbindgen_realloc: (a: number, b: number, c: number, d: number) => number;
  readonly __wbindgen_exn_store: (a: number) => void;
  readonly __wbindgen_thread_destroy: (a: number, b: number) => void;
  readonly __wbindgen_start: () => void;
}

After:

export interface InitOutput {
  readonly start: () => void;
  readonly run: () => void;
  readonly child_entry_point: (a: number) => void;
  readonly memory: WebAssembly.Memory;
  readonly __wbindgen_malloc: (a: number, b: number) => number;
  readonly __wbindgen_realloc: (a: number, b: number, c: number, d: number) => number;
  readonly __wbindgen_exn_store: (a: number) => void;
  readonly __wbindgen_thread_destroy: (a?: number, b?: number) => void;
  readonly __wbindgen_start: () => void;
}

Ideally I'd like better names too (a and b) but for the life of me I can't tell if it's in the wasm or not. Maybe just hardcode it since I'm already doing __wbindgen_thread_destroy? 🤷‍♂️ Oh well this is a strict improvement.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunately hacky :/

As far as I can tell we don't have more information available because we manually insert these functions into the module instead of exposing them with the wasm_bindgen macro. We could still actually add wasm-bindgen style descriptors manually to address this ... but it's questionable if that's worth it.

In any case, this is an improvement nevertheless, I don't see why not to take it.

Could you add a line to the changelog?

@AlexErrant
Copy link
Contributor Author

AlexErrant commented Nov 15, 2023

This is unfortunately hacky :/

Agreed, but not sure how else to make it cleaner 😅

Also added a changelog for #3690

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@daxpedda daxpedda merged commit fe88e39 into rustwasm:main Nov 15, 2023
@daxpedda
Copy link
Collaborator

Thank you!

@AlexErrant AlexErrant deleted the __wbindgen_thread_destroy_optional_args branch November 16, 2023 00:21
@linonetwo
Copy link

Hi, I'm searching for solution for #3818

May I ask, is this renamed to __wbindgen_free?

@daxpedda
Copy link
Collaborator

This function is made to destroy spawned threads, not the main instance.

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.

None yet

3 participants