Bug 195447 - [JSC] We should have more WithoutTransition functions which are usable for JSGlobalObject initialization
Summary: [JSC] We should have more WithoutTransition functions which are usable for JS...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
Keywords: InRadar
Depends on:
Reported: 2019-03-07 18:37 PST by Yusuke Suzuki
Modified: 2019-03-11 10:23 PDT (History)
10 users (show)

See Also:

Patch (78.46 KB, patch)
2019-03-07 22:42 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-03-07 18:37:36 PST
We are seeing transitions in JSGlobalObject initialization.
Comment 1 Yusuke Suzuki 2019-03-07 22:42:22 PST
Created attachment 363989 [details]
Comment 2 Filip Pizlo 2019-03-08 07:21:49 PST
Comment on attachment 363989 [details]

Comment 3 Yusuke Suzuki 2019-03-08 10:46:25 PST
Comment on attachment 363989 [details]

Comment 4 WebKit Commit Bot 2019-03-08 11:33:58 PST
Comment on attachment 363989 [details]

Clearing flags on attachment: 363989

Committed r242650: <https://trac.webkit.org/changeset/242650>
Comment 5 WebKit Commit Bot 2019-03-08 11:33:59 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2019-03-08 11:34:29 PST
Comment 7 Saam Barati 2019-03-11 10:23:49 PDT
Comment on attachment 363989 [details]

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

> Source/JavaScriptCore/runtime/JSObject.cpp:1933
> +    if (attributes & PropertyAttribute::ReadOnly)
> +        structure->setContainsReadOnlyProperties();

what would this even mean with an accessor? Perhaps this should be an assert that we're not read only?

> Source/JavaScriptCore/runtime/NullSetterFunction.h:38
> +        // Since NullSetterFunction is per JSGlobalObject, we use put-without-transition in InternalFunction::finishCreation.

This comment confuses me. You're using WithStructureTransition below, but "without" in this comment. Can you clarify what's going on? Can we just discard this comment since it seems contradictory to what the code is doing?