Last Comment Bug 737570 - ignore overwritten arguments via (non-toplevel) function statement named 'arguments'
: ignore overwritten arguments via (non-toplevel) function statement named 'arg...
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla14
Assigned To: general
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 724790
  Show dependency treegraph
Reported: 2012-03-20 12:08 PDT by Luke Wagner [:luke]
Modified: 2012-03-28 14:25 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix and test (3.65 KB, patch)
2012-03-20 12:08 PDT, Luke Wagner [:luke]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description User image Luke Wagner [:luke] 2012-03-20 12:08:11 PDT
Created attachment 607652 [details] [diff] [review]
fix and test

Broken by bug 724790.  Somehow no tests for this.  The full logic for deoptimizing functions containing non-toplevel function statements is, incredibly, split into 3 places.  This patch collects them in one place and explains the special case in Parser::statements().
Comment 1 User image Luke Wagner [:luke] 2012-03-24 14:56:13 PDT
Green on try.
Comment 2 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-03-26 15:21:56 PDT
Comment on attachment 607652 [details] [diff] [review]
fix and test

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

::: js/src/frontend/Parser.cpp
@@ +1656,5 @@
>              outertc->noteMightAliasLocals();
> +            outertc->noteHasExtensibleScope();
> +            outertc->flags |= TCF_FUN_HEAVYWEIGHT;
> +            if (fun->atom == context->runtime->atomState.argumentsAtom)
> +                outertc->flags |= TCF_FUN_LOCAL_ARGUMENTS;

Do we have a well-named helper for adding this bit?  If not, we probably should add one, so that the exact meaning and method of representation of the bit need not be remembered by the reader.
Comment 3 User image Luke Wagner [:luke] 2012-03-26 15:34:25 PDT
Righto.  noteLocalOverwritesArguments work for you?
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2012-03-26 15:44:48 PDT
Seems to work.
Comment 6 User image Ed Morley [:emorley] 2012-03-28 14:25:53 PDT

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