Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Mar 16, 2023

This primary change here is the creation of emscripten_internal.h
which is an internal-only header that contains that declaration of
internal JS function that are not already declared in other files.

The reason for this is to ensure that all JS library functions that are
callable form native code have a valid declaration, at least internally,
so that gen_sig_info.py can extract that signature from a compiled wasm
file.

This change add the scripte for generating library_sigs.js but doesn't
actually start using it, or erase the correpsonding hand-created __sig
entries. That is will be part of a followup

@sbc100 sbc100 force-pushed the gen_sig_info branch 2 times, most recently from 9cc746f to 0e7635b Compare March 17, 2023 17:32
@sbc100 sbc100 changed the title Automatically generate __sig properties for JS library symbols [WIP] Automatically generate __sig properties for JS library symbols Mar 17, 2023
sbc100 added a commit that referenced this pull request Mar 17, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 17, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 17, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 17, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 17, 2023
The emscripten_request_animation_frame sig was just wrong since that
Web API returns WebIDL `long` defined as i32.

For emscripten_webgl_get_parameter_i64v use GLint64 rather than `long
long` to me more explicit.

For `emscripten_websocket_get_buffered_amount` using `size_t` since we
already include `stdint.h` in this header anyway.

I found all of these while working on #18985 which detectes the use of
`long`, `size_t` or pointers and marks tham a `p` in their `__sig`
attribute.
sbc100 added a commit that referenced this pull request Mar 17, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 19, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
The emscripten_request_animation_frame sig was just wrong since that
Web API returns WebIDL `long` defined as i32.

For emscripten_webgl_get_parameter_i64v use GLint64 rather than `long
long` to me more explicit.

For `emscripten_websocket_get_buffered_amount` using `size_t` since we
already include `stdint.h` in this header anyway.

I found all of these while working on #18985 which detectes the use of
`long`, `size_t` or pointers and marks tham a `p` in their `__sig`
attribute.
sbc100 added a commit that referenced this pull request Mar 20, 2023
This change was 100% automatically generated by the tool I'm working on
as part of #18979.  Once this change lands, I plan to completely
remove these signatures and instead auto-generate them, but in order
to make that change into no-op I'd like to first update them all
in-place.

See #18985
sbc100 added a commit that referenced this pull request Mar 20, 2023
The emscripten_request_animation_frame sig was just wrong since that
Web API returns WebIDL `long` defined as i32.

For emscripten_webgl_get_parameter_i64v use GLint64 rather than `long
long` to me more explicit.

For `emscripten_websocket_get_buffered_amount` using `size_t` since we
already include `stdint.h` in this header anyway.

I found all of these while working on #18985 which detectes the use of
`long`, `size_t` or pointers and marks tham a `p` in their `__sig`
attribute.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 20, 2023
…root. NFC

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
sbc100 added a commit that referenced this pull request Mar 21, 2023
…root. NFC (#19013)

I noticed while working `gen_sig_info.py` in #18985 that is was simpler
to just keep a copy of the generated file checked into source control.
We can use a unit test check that its up-to-date on each commit.  In
fact we already had a unit test that was doing this and keeping a copy
of the expected output in the test directory.

This simplifies the emcc linker code since we can always assume the
presence of the struct info file.

Also allows us to remove the `varies=False` special case in cache logic.
@sbc100 sbc100 force-pushed the gen_sig_info branch 3 times, most recently from c658883 to 6d7ea2f Compare March 21, 2023 01:21
@sbc100 sbc100 force-pushed the gen_sig_info branch 3 times, most recently from ddaefc5 to ceb74e7 Compare March 21, 2023 18:57
@sbc100 sbc100 changed the title [WIP] Automatically generate __sig properties for JS library symbols Automatically generate __sig properties for JS symbols (part 1) Mar 21, 2023
@sbc100 sbc100 requested review from kripken March 21, 2023 18:58
@sbc100 sbc100 requested a review from dschuff March 21, 2023 19:17
@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 21, 2023

Gentle ping, if either of you get time today I'd love to land this so I can upload the followups.

@sbc100
Copy link
Collaborator Author

sbc100 commented Mar 21, 2023

Followup is here BTW: #19028

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Looks good enough to land and iterate to me.

This primary change here is the creation of `emscripten_internal.h`
which is an internal-only header that contains that declaration of
internal JS function that are not already declared in other files.

The reason for this is to ensure that all JS library functions that are
callable form native code have a valid declaration, at least internally,
so that gen_sig_info.py can extract that signature from a compiled wasm
file.

This change add the scripte for generating `library_sigs.js` but doesn't
actually start using it, or erase the correpsonding hand-created `__sig`
entries. That is will be part of a followup
@sbc100 sbc100 enabled auto-merge (squash) March 22, 2023 00:29
@sbc100 sbc100 merged commit 592aeca into main Mar 22, 2023
@sbc100 sbc100 deleted the gen_sig_info branch March 22, 2023 01:34
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.

3 participants