Last Comment Bug 737570 - ignore overwritten arguments via (non-toplevel) function statement named 'arguments'
: ignore overwritten arguments via (non-toplevel) function statement named 'arg...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: general
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description 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 Luke Wagner [:luke] 2012-03-24 14:56:13 PDT
Green on try.
Comment 2 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 Luke Wagner [:luke] 2012-03-26 15:34:25 PDT
Righto.  noteLocalOverwritesArguments work for you?
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2012-03-26 15:44:48 PDT
Seems to work.
Comment 6 Ed Morley [:emorley] 2012-03-28 14:25:53 PDT
https://hg.mozilla.org/mozilla-central/rev/31d88c5a7b18

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