Bug 199601 – JSGlobalObject type macros should support feature flags and WeakRef should have one

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

JSGlobalObject type macros should support feature flags and WeakRef should have one

[Keith Miller]

Created attachment 373683 [details]
Patch

[EWS Watchlist]

Comment on attachment 373683 [details]
Patch

Attachment 373683 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12694133

New failing tests:
js/weakref-async-is-collected.html
js/weakref-weakset-consistency.html
js/weakref-eventually-collects-values.html
js/weakref-microtasks-dont-collect.html

[EWS Watchlist]

Created attachment 373690 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 373683 [details]
Patch

Attachment 373683 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12694145

New failing tests:
js/weakref-async-is-collected.html
js/weakref-weakset-consistency.html
js/weakref-eventually-collects-values.html
js/weakref-microtasks-dont-collect.html

[EWS Watchlist]

Created attachment 373693 [details]
Archive of layout-test-results from ews116 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 373683 [details]
Patch

Attachment 373683 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12694573

New failing tests:
js/weakref-async-is-collected.html
js/weakref-weakset-consistency.html
js/weakref-eventually-collects-values.html
js/weakref-microtasks-dont-collect.html

[EWS Watchlist]

Created attachment 373696 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Keith Miller]

Created attachment 373701 [details]
Patch

[EWS Watchlist]

Comment on attachment 373701 [details]
Patch

Attachment 373701 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/12696454

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases
apiTests

[EWS Watchlist]

Comment on attachment 373701 [details]
Patch

Attachment 373701 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12696573

New failing tests:
js/weakref-async-is-collected.html
js/weakref-weakset-consistency.html
js/weakref-eventually-collects-values.html
js/weakref-microtasks-dont-collect.html

[EWS Watchlist]

Created attachment 373705 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Keith Miller]

Created attachment 374144 [details]
Patch

[Keith Miller]

Created attachment 374152 [details]
Patch

[Mark Lam]

Comment on attachment 374152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374152&action=review

r=me

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:721
> +#define CREATE_PROTOTYPE_FOR_LAZY_TYPE(capitalName, lowerName, properName, instanceType, jsName, prototypeBase, featureFlag) if (featureFlag) \

nit: I think you should add the { } since the body is a multi-line statement.

[Keith Miller]

Comment on attachment 374152 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374152&action=review

>> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:721
>> +#define CREATE_PROTOTYPE_FOR_LAZY_TYPE(capitalName, lowerName, properName, instanceType, jsName, prototypeBase, featureFlag) if (featureFlag) \
> 
> nit: I think you should add the { } since the body is a multi-line statement.

Done.

[Keith Miller]

Committed r247474: <https://trac.webkit.org/changeset/247474>

[Radar WebKit Bug Importer]

<rdar://problem/53137607>

[Ryan Haddad]

(In reply to Keith Miller from comment #16)
> Committed r247474: <https://trac.webkit.org/changeset/247474>
This change has caused 52 test262 failures:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20Test262%20%28Tests%29/builds/4317

[Keith Miller]

(In reply to Ryan Haddad from comment #18)
> (In reply to Keith Miller from comment #16)
> > Committed r247474: <https://trac.webkit.org/changeset/247474>
> This change has caused 52 test262 failures:
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20Test262%20%28Tests%29/builds/4317

Oh, those need to be re-baselined since the feature is off by default. We should really have a way to enable all experimental language features for test262 runs...

[Keith Miller]

(In reply to Ryan Haddad from comment #18)
> (In reply to Keith Miller from comment #16)
> > Committed r247474: <https://trac.webkit.org/changeset/247474>
> This change has caused 52 test262 failures:
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20Test262%20%28Tests%29/builds/4317

Should be re-baselined with https://trac.webkit.org/changeset/247485.