Bug 195447

Summary: [JSC] We should have more WithoutTransition functions which are usable for JSGlobalObject initialization
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, fpizlo, hi, joepeck, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

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]
Patch
Comment 2 Filip Pizlo 2019-03-08 07:21:49 PST 
Comment on attachment 363989 [details]
Patch

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

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

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 
<rdar://problem/48719776>
Comment 7 Saam Barati 2019-03-11 10:23:49 PDT 
Comment on attachment 363989 [details]
Patch

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?