Move catch prediction into frontend.

Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183
Committed: https://crrev.com/8f1f1cb1c7623c1906c7b25372b68fed1fa065e0
Cr-Commit-Position: refs/heads/master@{#37686}

[neis, 2016-07-12 13:51:40]

Description was changed from

==========
WIP catch prediction

BUG=
==========

to

==========
Move catch prediction into frontend.

And make it aware of rethrows, such as the one introduced by the for-of
desugaring.

BUG=v8:5183
==========

[neis, 2016-07-12 13:51:40]

neis@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[neis, 2016-07-12 13:52:40]

PTAL. I adapted Ignition and Full-codegen but not Crankshaft and Turbofan.

[Michael Starzinger, 2016-07-12 14:11:12]

Thanks! I'll take care of TurboFan in a follow-up CL.

https://codereview.chromium.org/2146493002/diff/20001/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/2146493002/diff/20001/src/ast/ast.h#newcode1152
src/ast/ast.h:1152: // although the handler may actually catch, but not the
other way around.
nit: The very last sentence of this comment is not 100% correct I think. The
following is a snippet where we predict "caught", even though the exception is
not "caught"

function f() {
  nope: {
    try {
      try {
        throw 23;
      } finally {
        break nope;
      }
    } catch(e) {
      // never hit!
    }
  }
}

Can we just rephrase the comment to be less precise? It's better to be less
precise instead of wrong IMHO. I was thinking along the lines of ...

// Note that this is only a conservative approximation, because it is
// undecidable whether a thrown exception is caught or not.

https://codereview.chromium.org/2146493002/diff/20001/src/full-codegen/full-c...
File src/full-codegen/full-codegen.h (right):

https://codereview.chromium.org/2146493002/diff/20001/src/full-codegen/full-c...
src/full-codegen/full-codegen.h:40: try_catch_depth_(0),
nit: This variable should no longer be needed, let's drop it.

https://codereview.chromium.org/2146493002/diff/20001/src/full-codegen/full-c...
src/full-codegen/full-codegen.h:972: int try_catch_depth_;
nit: This variable should no longer be needed, let's drop it.

https://codereview.chromium.org/2146493002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2146493002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:299: virtual ~ControlScopeForTryCatch() {}
nit: Class is final, implicit destructor is good enough, let's drop this line.

https://codereview.chromium.org/2146493002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:326: virtual ~ControlScopeForTryFinally()
{}
nit: Class is final, implicit destructor is good enough, let's drop this line.

https://codereview.chromium.org/2146493002/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.h (right):

https://codereview.chromium.org/2146493002/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.h:215: int try_finally_nesting_level_;
nit: Both of the local variables are no longer needed, let's drop them.

[neis, 2016-07-12 14:38:25]

Thanks, addressed all comments.

https://codereview.chromium.org/2146493002/diff/20001/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/2146493002/diff/20001/src/ast/ast.h#newcode1152
src/ast/ast.h:1152: // although the handler may actually catch, but not the
other way around.
On 2016/07/12 14:11:12, Michael Starzinger wrote:
> nit: The very last sentence of this comment is not 100% correct I think. The
> following is a snippet where we predict "caught", even though the exception is
> not "caught"
> 
> function f() {
>   nope: {
>     try {
>       try {
>         throw 23;
>       } finally {
>         break nope;
>       }
>     } catch(e) {
>       // never hit!
>     }
>   }
> }

Yes, you are right.  I reworded and removed the word "conservative" since it's
misleading.

[neis, 2016-07-12 14:38:30]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 14:38:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[neis, 2016-07-12 14:38:46]

neis@chromium.org changed reviewers:
+ yangguo@google.com

[neis, 2016-07-12 14:39:03]

neis@chromium.org changed reviewers:
+ yangguo@chromium.org
- yangguo@google.com

[Michael Starzinger, 2016-07-12 14:43:25]

LGTM. Thanks!

[commit-bot: I haz the power, 2016-07-12 15:08:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-12 15:08:49]

Dry run: This issue passed the CQ dry run.

[neis, 2016-07-12 15:17:40]

Description was changed from

==========
Move catch prediction into frontend.

And make it aware of rethrows, such as the one introduced by the for-of
desugaring.

BUG=v8:5183
==========

to

==========
Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183
==========

[neis, 2016-07-12 15:18:14]

The CQ bit was checked by neis@chromium.org

[commit-bot: I haz the power, 2016-07-12 15:18:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 15:20:46]

Description was changed from

==========
Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183
==========

to

==========
Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183
==========

[commit-bot: I haz the power, 2016-07-12 15:20:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-12 15:20:49]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-12 15:23:42]

Description was changed from

==========
Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183
==========

to

==========
Move catch prediction into frontend and make it aware of rethrows.

This solves an issue with throws inside for-of always being marked as caught.

BUG=v8:5183

Committed: https://crrev.com/8f1f1cb1c7623c1906c7b25372b68fed1fa065e0
Cr-Commit-Position: refs/heads/master@{#37686}
==========

[commit-bot: I haz the power, 2016-07-12 15:23:43]

Patchset 4 (id:??) landed as
https://crrev.com/8f1f1cb1c7623c1906c7b25372b68fed1fa065e0
Cr-Commit-Position: refs/heads/master@{#37686}

[Yang, 2016-07-13 08:39:58]

This is awesome! Belated LGTM.