Last Comment Bug 640593 - Remove fun.arity
: Remove fun.arity
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla7
Assigned To: Tom Schuster [:evilpie]
: Jason Orendorff [:jorendorff]
: 569172 (view as bug list)
Depends on: 666095 666587 667538 696342
Blocks: 666733
  Show dependency treegraph
Reported: 2011-03-10 06:48 PST by Tom Schuster [:evilpie]
Modified: 2011-10-21 08:33 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

remove fun.arity and other cleanup related to function props (10.35 KB, patch)
2011-05-29 03:19 PDT, Tom Schuster [:evilpie]
no flags Details | Diff | Splinter Review
removed tabs, same patch as above (10.19 KB, patch)
2011-05-29 03:28 PDT, Tom Schuster [:evilpie]
no flags Details | Diff | Splinter Review
v2 (13.92 KB, patch)
2011-05-31 07:01 PDT, Tom Schuster [:evilpie]
jorendorff: review+
Details | Diff | Splinter Review

Description User image Tom Schuster [:evilpie] 2011-03-10 06:48:57 PST
Both IE and Chrome don't have it.
Comment 1 User image David Bruant 2011-05-29 02:12:45 PDT
And it's not standard and fun.length is the standard replacement for that.
Comment 2 User image Tom Schuster [:evilpie] 2011-05-29 03:19:13 PDT
Created attachment 535915 [details] [diff] [review]
remove fun.arity and other cleanup related to function props

So took my chances here.
- Removed lazyDataProps and defined name the same way as length in resolve
- Stop using tinyids (i would like to remove those at some point)
- Made arguments/caller behave as expected, previously they wouldn't be resolved from the proto chain
Comment 3 User image Tom Schuster [:evilpie] 2011-05-29 03:28:00 PDT
Created attachment 535916 [details] [diff] [review]
removed tabs, same patch as above

thanks Ms2ger
Comment 4 User image Tom Schuster [:evilpie] 2011-05-31 07:01:47 PDT
Created attachment 536287 [details] [diff] [review]

updated to pass test suite
Comment 5 User image Jason Orendorff [:jorendorff] 2011-06-17 08:35:28 PDT
Comment on attachment 536287 [details] [diff] [review]

Review of attachment 536287 [details] [diff] [review]:

Love it. r=me with the two changes below.

For future reference -- hg mv is better for renaming a file, since it keeps the file's history intact, and the patch would actually show the diffs for review instead of a whole file being removed and a whole new file being added.

::: js/src/jsfun.cpp
@@ +1604,3 @@
>  struct PoisonPillProp {
>      uint16       atomOffset;

Go ahead and get rid of this struct. Make poisonPillProps an array of uint16. Feel free to get rid of it entirely and turn the loop in fun_resolve into a single if-statement, like the length||name one you added.

@@ +2834,5 @@
>       * parameter, and so binds fun's parent to obj using JSObject::setParent,
>       * under js_NewFunction (in JSObject::init, called from NewObject -- see
>       * jsobjinlines.h).
>       *
> +     * But JSObject::setParent sets the DELEGATE object flag on its receiver,z

Comment 6 User image Tom Schuster [:evilpie] 2011-06-18 05:06:40 PDT
Comment 7 User image Chris Leary [:cdleary] (not checking bugmail) 2011-06-20 17:10:58 PDT
cdleary-bot mozilla-central merge info:
Comment 8 User image Eric Shepherd [:sheppy] 2011-06-21 06:51:59 PDT
Documentation updated by evilpie:

And mentioned on Firefox 7 for developers.
Comment 9 User image Tom Schuster [:evilpie] 2011-06-22 13:46:42 PDT
This was
Comment 10 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-24 10:47:47 PDT
So what did this patch change other than removing .arity?  At least one of the bugs that this change caused appears on a web page that doesn't appear to use .arity....
Comment 11 User image David Bruant 2011-06-24 10:58:38 PDT
See comment 2
Comment 12 User image David Bruant 2011-06-24 11:03:55 PDT
Massive use of arguments in the dropbox zip from bug 666587
Comment 13 User image Tom Schuster [:evilpie] 2011-06-27 16:13:07 PDT
*** Bug 569172 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.