Handle switch effects

Handle switch effects

R=titzer@chromium.org
BUG=

Committed: http://code.google.com/p/v8/source/detail?r=15777

[titzer, 2013-07-15 09:18:18]

https://codereview.chromium.org/18926004/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/18926004/diff/1/src/ast.h#newcode1038
src/ast.h:1038: class ContinueStatement: public JumpStatement {
You could also just implement IsJump() in the subclasses.

https://codereview.chromium.org/18926004/diff/1/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/18926004/diff/1/src/typing.cc#newcode181
src/typing.cc:181: local_effects.Alt(clause_effects);
I'm not sure this is quite right. Did you define IsJump() to be true for both
break and continue?

[rossberg, 2013-07-15 09:44:59]

https://codereview.chromium.org/18926004/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/18926004/diff/1/src/ast.h#newcode1038
src/ast.h:1038: class ContinueStatement: public JumpStatement {
On 2013/07/15 09:18:18, titzer wrote:
> You could also just implement IsJump() in the subclasses.

Yeah, but I thought this could be conceptually useful. But maybe not. Does it
harm, though?

https://codereview.chromium.org/18926004/diff/1/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/18926004/diff/1/src/typing.cc#newcode181
src/typing.cc:181: local_effects.Alt(clause_effects);
On 2013/07/15 09:18:18, titzer wrote:
> I'm not sure this is quite right. Did you define IsJump() to be true for both
> break and continue?

Yes, see ast.h. The idea is to give up on non-trivial fall-through, at least for
the time being. What precisely worries you?

[titzer, 2013-07-15 13:30:42]

https://codereview.chromium.org/18926004/diff/1/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/18926004/diff/1/src/typing.cc#newcode181
src/typing.cc:181: local_effects.Alt(clause_effects);
On 2013/07/15 09:44:59, rossberg wrote:
> On 2013/07/15 09:18:18, titzer wrote:
> > I'm not sure this is quite right. Did you define IsJump() to be true for
both
> > break and continue?
> 
> Yes, see ast.h. The idea is to give up on non-trivial fall-through, at least
for
> the time being. What precisely worries you?

So you actually have 5 different types of control transfer:

1. statement has no control flow => 
2. statement is break, inside a case => goes to end of switch
3. statement is break, inside a loop => goes to end of loop
4. statement is continue => goes to start of loop
5. statement is throw => goes out of method or to catch

Compositionally, a compound statement (if-then-else, loop, switch) could do any
set of these.

I'm not sure which of the behaviors you want to handle here, or what you want to
do for each.

[rossberg, 2013-07-15 14:21:56]

https://codereview.chromium.org/18926004/diff/1/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/18926004/diff/1/src/typing.cc#newcode181
src/typing.cc:181: local_effects.Alt(clause_effects);
On 2013/07/15 13:30:42, titzer wrote:
> On 2013/07/15 09:44:59, rossberg wrote:
> > On 2013/07/15 09:18:18, titzer wrote:
> > > I'm not sure this is quite right. Did you define IsJump() to be true for
> both
> > > break and continue?
> > 
> > Yes, see ast.h. The idea is to give up on non-trivial fall-through, at least
> for
> > the time being. What precisely worries you?
> 
> So you actually have 5 different types of control transfer:
> 
> 1. statement has no control flow => 
> 2. statement is break, inside a case => goes to end of switch
> 3. statement is break, inside a loop => goes to end of loop
> 4. statement is continue => goes to start of loop
> 5. statement is throw => goes out of method or to catch

There's also 'return', and a 'continue' in a 'for' loop, which behaves
differently from other loops. But I don't think any of that matters.

> Compositionally, a compound statement (if-then-else, loop, switch) could do
any
> set of these.
> 
> I'm not sure which of the behaviors you want to handle here, or what you want
to
> do for each.

Where the control goes does not matter, as long as it's out of the switch (which
is true for all of the above). All you want to ensure is that control from one
case cannot flow into the next case (a.k.a. fall-through). That is, I want a
well-behaved switch that is just a multiple branch, and no random mess. Because
only then all the branch effects are mutually exclusive.

[titzer, 2013-07-16 09:14:04]

On 2013/07/15 14:21:56, rossberg wrote:
> https://codereview.chromium.org/18926004/diff/1/src/typing.cc
> File src/typing.cc (right):
> 
> https://codereview.chromium.org/18926004/diff/1/src/typing.cc#newcode181
> src/typing.cc:181: local_effects.Alt(clause_effects);
> On 2013/07/15 13:30:42, titzer wrote:
> > On 2013/07/15 09:44:59, rossberg wrote:
> > > On 2013/07/15 09:18:18, titzer wrote:
> > > > I'm not sure this is quite right. Did you define IsJump() to be true for
> > both
> > > > break and continue?
> > > 
> > > Yes, see ast.h. The idea is to give up on non-trivial fall-through, at
least
> > for
> > > the time being. What precisely worries you?
> > 
> > So you actually have 5 different types of control transfer:
> > 
> > 1. statement has no control flow => 
> > 2. statement is break, inside a case => goes to end of switch
> > 3. statement is break, inside a loop => goes to end of loop
> > 4. statement is continue => goes to start of loop
> > 5. statement is throw => goes out of method or to catch
> 
> There's also 'return', and a 'continue' in a 'for' loop, which behaves
> differently from other loops. But I don't think any of that matters.
> 
> > Compositionally, a compound statement (if-then-else, loop, switch) could do
> any
> > set of these.
> > 
> > I'm not sure which of the behaviors you want to handle here, or what you
want
> to
> > do for each.
> 
> Where the control goes does not matter, as long as it's out of the switch
(which
> is true for all of the above). All you want to ensure is that control from one
> case cannot flow into the next case (a.k.a. fall-through). That is, I want a
> well-behaved switch that is just a multiple branch, and no random mess.
Because
> only then all the branch effects are mutually exclusive.

LGTM since you convinced me this code is exploratory, but it's looking more and
more like a proper SSA renaming pass is coming in the future.

[rossberg, 2013-07-19 13:04:47]

Committed patchset #2 manually as r15777 (presubmit successful).