Bug 154604 - Function.name and Function.length should be configurable.
Summary: Function.name and Function.length should be configurable.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-02-23 13:49 PST by Mark Lam
Modified: 2016-02-26 14:31 PST (History)
7 users (show)

See Also:


Attachments
work in progress: changed 'name' and 'length' properties to be reified on access. (11.89 KB, patch)
2016-02-25 13:44 PST, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (222.44 KB, patch)
2016-02-26 11:49 PST, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch: rebased. (222.44 KB, patch)
2016-02-26 11:53 PST, Mark Lam
saam: review+
Details | Formatted Diff | Diff
proposed patch 2: addressed Saam's feedback w/ new test. (224.53 KB, patch)
2016-02-26 14:19 PST, Mark Lam
saam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.