Skip to content

__wbindgen_thread_destroy has optional params#3703

Merged
daxpedda merged 3 commits intowasm-bindgen:mainfrom
AlexErrant:__wbindgen_thread_destroy_optional_args
Nov 15, 2023
Merged

__wbindgen_thread_destroy has optional params#3703
daxpedda merged 3 commits intowasm-bindgen:mainfrom
AlexErrant:__wbindgen_thread_destroy_optional_args

Conversation

@AlexErrant
Copy link
Copy Markdown
Contributor

@AlexErrant AlexErrant commented Nov 12, 2023 •
edited
Loading

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

Ref: https://github.com/rustwasm/wasm-bindgen/blob/9fb3bca16876c756266274f78fcd0214e0581eaa/crates/threads-xform/src/lib.rs#L394-L395

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.

daxpedda
daxpedda requested changes Nov 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@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
Copy Markdown
Contributor Author

AlexErrant commented Nov 15, 2023 •
edited
Loading

This is unfortunately hacky :/

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

Also added a changelog for #3690

daxpedda
daxpedda requested changes Nov 15, 2023
View reviewed changes
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
@daxpedda daxpedda merged commit fe88e39 into wasm-bindgen:main Nov 15, 2023
@daxpedda
Copy link
Copy Markdown
Member

daxpedda commented Nov 15, 2023

Thank you!

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

linonetwo commented Jan 31, 2024

Hi, I'm searching for solution for #3818

May I ask, is this renamed to __wbindgen_free?

@daxpedda
Copy link
Copy Markdown
Member

daxpedda commented Jan 31, 2024

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

Reviewers

@daxpedda daxpedda daxpedda requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AlexErrant @daxpedda @linonetwo