Bug 154604

Summary: Function.name and Function.length should be configurable.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, keith_miller, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
work in progress: changed 'name' and 'length' properties to be reified on access.
none
proposed patch.
none
proposed patch: rebased.
saam: review+
proposed patch 2: addressed Saam's feedback w/ new test. saam: review+

Description Mark Lam 2016-02-23 13:49:20 PST
According to https://tc39.github.io/ecma262/#sec-ecmascript-language-functions-and-classes, "Unless otherwise specified, the name property of a built-in Function object, if it exists, has the attributes { [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: true }."
Comment 1 Radar WebKit Bug Importer 2016-02-23 13:51:50 PST
<rdar://problem/24799540>
Comment 2 Mark Lam 2016-02-25 13:44:43 PST
Created attachment 272233 [details]
work in progress: changed 'name' and 'length' properties to be reified on access.
Comment 3 Mark Lam 2016-02-26 11:48:11 PST
https://tc39.github.io/ecma262/#sec-ecmascript-language-functions-and-classe also says "the length property of a built-in Function object has the attributes { [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: true }."
Comment 4 Mark Lam 2016-02-26 11:49:38 PST
Created attachment 272353 [details]
proposed patch.
Comment 5 Mark Lam 2016-02-26 11:53:22 PST
Created attachment 272354 [details]
proposed patch: rebased.
Comment 6 Saam Barati 2016-02-26 12:31:02 PST
Comment on attachment 272354 [details]
proposed patch: rebased.

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

r=me with comment

> Source/JavaScriptCore/runtime/JSFunction.cpp:522
> +        if (propertyName == exec->propertyNames().length)
> +            thisObject->reifyLengthIfNeeded(exec);
> +        else if (propertyName == exec->propertyNames().name)
> +            thisObject->reifyNameIfNeeded(exec);

This code is repeated a lot. It might be worth having an inlined helper here that reifies if necessary.
You could call it reifyIfNecessary(PropertyName) or something.
Comment 7 Saam Barati 2016-02-26 12:31:44 PST
It might be worth adding some tests that will gracefully handle structure transitions
after reification.
Comment 8 Saam Barati 2016-02-26 12:40:39 PST
It's also worth adding tests that do indeed verify that
the property is configurable.
Comment 9 Mark Lam 2016-02-26 14:19:48 PST
Created attachment 272371 [details]
proposed patch 2: addressed Saam's feedback w/ new test.
Comment 10 Mark Lam 2016-02-26 14:31:15 PST
Thanks for the review.  Landed in r197205: <http://trac.webkit.org/r197205>.