1223652 - Remove else after return from CGBlockScopeList::findEnclosingScope.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

In CGBlockScopeList::findEnclosingScope, |else| clause is placed after |return| statement, we can remove the |else {| and corresponding |}| to reduce the indent of that block.

https://dxr.mozilla.org/mozilla-central/rev/cc473fe5dc512c450634506f68cbacfb40a06a23/js/src/frontend/BytecodeEmitter.cpp#8438
>         if (list[index].end == 0) {
>             // We are looking for the nearest enclosing live scope.  If the
>             // scope contains POS, it should still be open, so its length should
>             // be zero.
>             return list[index].index;
>         } else {
>             // Conversely, if the length is not zero, it should not contain
>             // POS.
>             MOZ_ASSERT_IF(inPrologue == list[index].endInPrologue, list[index].end <= pos);
>         }

Required code changes are following:
  * Remove else after return from CGBlockScopeList::findEnclosingScope above
  * reduce the indent of that block
  * re-wrap the comment there to fit to 79 chars

You'll need basic knowledge of C++, and be able to build and test Firefox [1] or SpiderMonkey [2].

A skilled first-time contributor should be able to finish this in 2 weeks.  Leave comments / questions here, or ask me (:arai) in IRC #introduction and #jsapi channels.

[1] https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation

Comment 1

[Nicholas Parkanyi]

Attachment: Removes the redundant else block in CGBlockScopeList::findEnclosingScope.

Comment 2

[Tooru Fujisawa [:arai]]

Comment on attachment 8686336 [details] [diff] [review]
Removes the redundant else block in CGBlockScopeList::findEnclosingScope.

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

Thank you for your patch :D
Excellent, it's perfect!

Here's a try run (it builds and runs automated tests) for your patch:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b65c241b944
After it runs without any error, your patch can be landed.

By the way, have you built SpiderMonkey (or Firefox) and tested your patch locally?
If not yet, it will be a good exercise, give it a try :)
  https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation#Building_SpiderMonkey
  https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Build_Documentation#Test

Comment 3

[Tooru Fujisawa [:arai]]

Try run is green :)

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bed0a37b59a6

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bed0a37b59a6