[WebIDL] Change generated statics to be accessed in the way the spec specifies #8706

VirtualTim · 2019-05-30T06:21:21Z

According to the WebIDL spec static properties should be accessed like [class].[function], not [class].prototype.[function].
See: https://www.w3.org/TR/WebIDL/#idl-static-attributes-and-operations

Fixes #8705.

…ecifies According to the WebIDL spec static properties should be accessed like [class].[function], not [class]prototype.[function]. See: https://www.w3.org/TR/WebIDL/#idl-static-attributes-and-operations

kripken · 2019-06-03T16:50:33Z

I'm not familiar enough with WebIDL, but we have sort of assumed that "static" meant what it means in C/C++. It might be accurate either either way, though, assuming people don't manually change the prototype themselves.

However, the test fails on CI here, so perhaps something is wrong?

VirtualTim · 2019-06-04T01:51:59Z

Yeah I just missed updating some tests. Hopefully that's all of them.
I'm not really sure how things should work under the surface, the the spec (surprisingly) seems pretty clear on how statics should be accessed.

Here's a cut down version of the example taken from the spec link posted above.

IDL:

interface Circle {
  static Point triangulate(Circle c1, Circle c2, Circle c3);
};

JS	Current Behaviour	Behaviour according to the spec
Circle.prototype.triangulate	executes triangulate	Evaluates to undefined
Circle.triangulate(...)	Evaluates to undefined	executes triangulate

VirtualTim · 2019-06-04T02:38:41Z

So I don't know why the filesize tests are now failing.
The succeeding run printed:

Running metadce test: minimal.c: [] 11 [] ['waka'] 9211 5 13 16
  seen wasm size: 8033 (expected: 7871), ratio to expected: 0.020582
test_binaryen_metadce_minimal_O2 (test_other.other) ... ok

The failing run printed:

Running metadce test: minimal.c: ['-O1'] 9 [] ['waka'] 7886 2 12 10
  seen wasm size: 8011 (expected: 7871), ratio to expected: 0.017787
test_binaryen_metadce_minimal_O2 (test_other.other) ... FAIL

kripken · 2019-06-05T16:50:00Z

tests/webidl/post.js

 console.log(succeeded);

-TheModule.Child2.prototype.printStatic(); // static calls go through the prototype
+TheModule.Child2.printStatic(); // static calls do not go through the prototype


would this still work with .prototype as well? if yes, then we should test both. if not, then it's a breaking change, and maybe not worth it.

The old approach should work with or without .prototype, I think, so if we decide to not change this, maybe we can at least add testing and docs for not writing .prototype

When I try and call something like MyModule.CConfig.SetStaticOption(...) I get (in Chrome) Uncaught TypeError: MyModule.CConfig.SetStaticOption is not a function.

I would have thought for this to work I would have needed to instantiate a CConfig option. Right?

The proposed change still works like MyModule.CConfig.prototype.SetStaticOption(...), but can now also work like MyModule.CConfig.SetStaticOption(...).

I see, thanks, sounds good.

I think that leaves testing - as mentioned earlier, please both both notations that work after this change.

I updated the tests to test both access methods, but the below line (TheModule.Child2.prototype.printStatic();) seems to fail on node in test-uvwxyz. It looks like it must be throwing? I don't have any node experience, any ideas what could be going wrong?

Also this appears to have increased the number of exports by one in the test-upstream-other tests. But I'm not really sure what the significance of that is, or why my change affected that?
Any pointers?
Edit: fixed after merging in latest. No idea how that broke or was fixed.

kripken · 2019-06-26T20:09:11Z

cc @wingo - would be good if you can take a look here, thanks!

kripken · 2019-07-02T21:44:49Z

I think the metadce test errors in other are due to a temporary breakage we had earlier - linking in latest incoming should fix that.

I don't have any ideas about what could be wrong in the webidl test error. Does it fail for you locally when running that test?

kripken · 2019-07-08T21:26:28Z

The test_webidl error here looks real.

VirtualTim · 2019-07-09T07:01:23Z

The test_webidl error here looks real.

Yep, just haven't got around to looking at it yet. TheModule.Child2.printStatic(); works, but TheModule.Child2.prototype.printStatic(); fails.

Personally I think we should drop accessing statics like TheModule.Child2.prototype.xxx, since that is directly against the spec (technically should evaluate to undefined), but if you want it in I've got to fix it.

Edit: crap, just noticed I messed up the merge, I'll fix that too.

This reverts commit 50d6ff5.

Revert "Merge in incoming changes"

Re-merge latest incoming

VirtualTim · 2019-07-18T02:05:28Z

@kripken I'm going to need some help understanding the failing test. It looks like the only one caused directly by me is under test-uvwxyz.
It fails on test_webidl (test_core.asm2), but succeeds on all the other test_webidl (test_core.[])s
I'm not sure how to interpret this. Does this mean that it works everywhere except node 8.9.1?
I also cant spot where the test is actually compiled. I thought it would be under test_webidl, but I can't see it? Also what do those asm2, asmi, asm2f, etc mean?

kripken · 2019-07-19T18:16:48Z

@VirtualTim looks like it fails on FAIL: test_webidl (test_core.asm2) which means asm2.test_webidl (runnable locally with ./tests/runner.py asm2.test_webidl). asm2 is defined here:

emscripten/tests/test_core.py

Line 7998 in ab4953c

asm2 = make_run('asm2', emcc_args=['-O2'], settings={'WASM': 0})

it's basically asm.js with -O2. That runs on fastcomp, not upstream, btw (can see the job info in .circleci/config.yml).

The actual building is done by do_run() which is called in test_webidl. The implementation is in tests/runner.py. Some debugging notes here on running locally: https://emscripten.org/docs/getting_started/test-suite.html#debugging-test-failures Running with EMCC_DEBUG=1 locally is probably the most straightforward way to see what emcc command is run (it logs "invocation" and the full command).

It does mention the test failed on node, but that's just because it was the first JS engine to be tested, and it failed - so nothing special about node there, I'm pretty sure. (once one fails, it stops checking other JS VMs on that test)

VirtualTim · 2019-07-22T05:18:46Z

Thanks for that. I think the problem is that tests/webidl/post.js runs some commands directly, when they should probably be inside a Module['onRuntimeInitialized'] = function() {...}. Not 100% sure that's what the issue is, but I've created a separate PR (#9049) to experiment.

But I don't know why no one hasn't seen this before, or why my change caused issues.

kripken · 2019-07-24T16:53:19Z

Does that PR fix the issue @VirtualTim? My guess it may be unrelated, but I could be wrong.

Let me know if you need help debugging this.

VirtualTim · 2019-07-25T01:48:08Z

Sorry, haven't had much of chance to spend time on this. The RP didn't help, I think I must have had some misunderstanding abut the code the test generated.
It's really odd that this only fails with -O2, I thought it was because some calls could happen before the runtime was initialised, but I'm now unsure if that's the case.

The fact that it only fails with -O2 makes me think that this isn't anything I've introduced, but I'd like to figure it out before this PR lands just to be sure.

kripken · 2019-07-26T17:20:06Z

Some -O2 tests do additional things like run closure compiler - so maybe the bug here is specific to closure. Building with closure and -g1 (to not minify whitespace) may help.

I'd still like to merge this, but haven't had the time to work on it. May as well merge in incoming to keep this up to date.

sbc100 · 2020-01-30T19:43:18Z

The deletion of the incoming branch triggered the automatic closing of this branch. My apologies. This was an unexpected side effect.

stale · 2021-01-31T00:59:23Z

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

VirtualTim · 2021-02-01T03:56:29Z

I still think this would be nice, but could never figure out that failing test...
I don't really have time to work on this at the moment, but hopefully I'll be able to pick this up one day.

stale · 2022-04-16T15:21:41Z

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

VirtualTim · 2022-06-13T05:42:35Z

I just merged in the latest Emscription changes to see if that changed anything.
Unfortunately I still get the same error in test-core2: TypeError: TheModule.Child2.prototype.printStatic is not a function. It's very odd that this does not occur in test-core0 or test-core3.

I still don't have any idea how to fix this, but maybe I'll try again in another few years.

sbc100 · 2025-02-27T20:05:23Z

Do we still want to try and land this?

sbc100 · 2025-08-01T21:11:49Z

Closing this for now as inactive. Feel free to re-open.

VirtualTim added 4 commits May 30, 2019 14:15

Change WebIDL generated statics to be accessed in the way the spec sp…

c09424f

…ecifies According to the WebIDL spec static properties should be accessed like [class].[function], not [class]prototype.[function]. See: https://www.w3.org/TR/WebIDL/#idl-static-attributes-and-operations

Change test to reflect change in behaviour

0c8508b

Remove line that was meant to be replaced

1c4edbc

Update comment

2691082

sbc100 approved these changes May 30, 2019

View reviewed changes

Update some missed tests to reflect change in behaviour.

333ecc2

kripken reviewed Jun 5, 2019

View reviewed changes

VirtualTim changed the title ~~Change WebIDL generated statics to be accessed in the way the spec specifies~~ Jun 26, 2019

Change WebIDL tests to also test accessing them off the prototype

bdfe7bd

VirtualTim mentioned this pull request Jun 26, 2019

[webidl] Use sequences instead of arrays #8849

Closed

VirtualTim added 3 commits June 26, 2019 13:32

Change test output for additional tests (all)

877b62d

Change test output for additional tests (default)

326b37a

Change test output for additional tests (fast)

0531f8e

Merge in incoming changes

50d6ff5

VirtualTim added 3 commits July 9, 2019 15:06

Revert "Merge in incoming changes"

48ae205

This reverts commit 50d6ff5.

Merge pull request #6 from VirtualTim/revert-5-incoming

6de332f

Revert "Merge in incoming changes"

Merge pull request #7 from emscripten-core/incoming

a3e6784

Re-merge latest incoming

VirtualTim added 6 commits October 17, 2019 09:45

Merge in incoming changes

59a4b1a

I'd still like to merge this, but haven't had the time to work on it. May as well merge in incoming to keep this up to date.

Update shell.js

7882ce3

Add environment ifdefs

463566b

Update shell.js

aaa7bec

Update preprocessor.js

407fbf6

Revert changes accidentally committed to wrong branch

092166e

sbc100 closed this Jan 30, 2020

sbc100 reopened this Jan 30, 2020

sbc100 changed the base branch from incoming to master January 30, 2020 20:03

stale bot added the wontfix label Jan 31, 2021

stale bot removed the wontfix label Feb 1, 2021

Base automatically changed from master to main March 8, 2021 23:49

stale bot added the wontfix label Apr 16, 2022

stale bot closed this Jun 12, 2022

Merge branch 'emscripten-core:main' into patch-2

ec739f0

VirtualTim reopened this Jun 13, 2022

stale bot removed the wontfix label Jun 13, 2022

sbc100 closed this Aug 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WebIDL] Change generated statics to be accessed in the way the spec specifies #8706

[WebIDL] Change generated statics to be accessed in the way the spec specifies #8706

Uh oh!

VirtualTim commented May 30, 2019

kripken commented Jun 3, 2019

VirtualTim commented Jun 4, 2019

VirtualTim commented Jun 4, 2019

kripken Jun 5, 2019

kripken Jun 5, 2019

VirtualTim Jun 10, 2019 •

edited

Loading

kripken Jun 11, 2019

kripken Jun 11, 2019

VirtualTim Jun 26, 2019 •

edited

Loading

kripken commented Jun 26, 2019

kripken commented Jul 2, 2019

kripken commented Jul 8, 2019

VirtualTim commented Jul 9, 2019 •

edited

Loading

VirtualTim commented Jul 18, 2019

kripken commented Jul 19, 2019

VirtualTim commented Jul 22, 2019 •

edited

Loading

kripken commented Jul 24, 2019

VirtualTim commented Jul 25, 2019

kripken commented Jul 26, 2019

sbc100 commented Jan 30, 2020

stale bot commented Jan 31, 2021

VirtualTim commented Feb 1, 2021

stale bot commented Apr 16, 2022

VirtualTim commented Jun 13, 2022

sbc100 commented Feb 27, 2025

sbc100 commented Aug 1, 2025

Labels

3 participants

[WebIDL] Change generated statics to be accessed in the way the spec specifies #8706

[WebIDL] Change generated statics to be accessed in the way the spec specifies #8706

Uh oh!

Conversation

VirtualTim commented May 30, 2019

kripken commented Jun 3, 2019

VirtualTim commented Jun 4, 2019

VirtualTim commented Jun 4, 2019

kripken Jun 5, 2019

Choose a reason for hiding this comment

kripken Jun 5, 2019

Choose a reason for hiding this comment

VirtualTim Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

kripken Jun 11, 2019

Choose a reason for hiding this comment

kripken Jun 11, 2019

Choose a reason for hiding this comment

VirtualTim Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

kripken commented Jun 26, 2019

kripken commented Jul 2, 2019

kripken commented Jul 8, 2019

VirtualTim commented Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

VirtualTim commented Jul 18, 2019

kripken commented Jul 19, 2019

VirtualTim commented Jul 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kripken commented Jul 24, 2019

VirtualTim commented Jul 25, 2019

kripken commented Jul 26, 2019

sbc100 commented Jan 30, 2020

stale bot commented Jan 31, 2021

VirtualTim commented Feb 1, 2021

stale bot commented Apr 16, 2022

VirtualTim commented Jun 13, 2022

sbc100 commented Feb 27, 2025

sbc100 commented Aug 1, 2025

Labels

3 participants

VirtualTim Jun 10, 2019 •

edited

Loading

VirtualTim Jun 26, 2019 •

edited

Loading

VirtualTim commented Jul 9, 2019 •

edited

Loading

VirtualTim commented Jul 22, 2019 •

edited

Loading