Last Comment Bug 765976 - Simplify CompileFunctionBody's control flow
: Simplify CompileFunctionBody's control flow
Status: RESOLVED FIXED
[js:t]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-18 16:58 PDT by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2012-06-20 02:18 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Simplify CompileFunctionBody's control flow (3.94 KB, patch)
2012-06-18 16:58 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
sphink: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-18 16:58:13 PDT
CompileFunctionBody()'s second half is oddly keen to avoid return
statements.  By using them, this patch reduces the nesting greatly and makes
the function more readable and more like CompileScript() (and most other
SpiderMonkey code).
Comment 1 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-18 16:58:24 PDT
Created attachment 634241 [details] [diff] [review]
Simplify CompileFunctionBody's control flow
Comment 2 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-18 17:00:14 PDT
Hey, |hg bzexport| actually works!  Nice.
Comment 3 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-18 17:03:53 PDT
BTW, I removed the "/* FoldConstants reported the error already. */" comment because AFAICT it's not true -- FoldConstants() doesn't report any errors.
Comment 4 Steve Fink [:sfink] [:s:] 2012-06-19 09:35:39 PDT
Comment on attachment 634241 [details] [diff] [review]
Simplify CompileFunctionBody's control flow

Review of attachment 634241 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, yes, a thousand times yes. The original was... well... WHY??!
Comment 5 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-06-19 17:31:51 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dba7486e5c7
Comment 6 Ed Morley [:emorley] 2012-06-20 02:18:54 PDT
https://hg.mozilla.org/mozilla-central/rev/0dba7486e5c7

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