<span style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">Maurizio-</span><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><br></div><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">

Overall, nice work.  I did a similar refactoring recently on another code base and was able to extract much more of the code (e.g. most of the code handling statements) into a common base class by abstracting over the type of the state variable, but it isn&#39;t clear that would be a win here.</div>

<div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><br></div><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">It would be helpful to have comments at the top of FlowAnalyzer and AssignAnalyzer explaining what they do (i.e. the division of labor).</div>

<div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><br></div><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">I suggest moving the assignment to finallyCanCompleteNormally outside of its if statement to make it clear that it is being given its final value, like so:</div>

<br style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><blockquote style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255);margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:40px;border-top-style:none;border-right-style:none;border-bottom-style:none;border-left-style:none;border-width:initial;border-color:initial;padding-top:0px;padding-right:0px;padding-bottom:0px;padding-left:0px">

tree.finallyCanCompleteNormally = alive;</blockquote><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)"><br></div><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">

Cheers,</div><div style="font-family:arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">Neal</div><br><div class="gmail_quote">On Fri, Mar 9, 2012 at 3:00 AM, Maurizio Cimadamore <span dir="ltr">&lt;<a href="mailto:maurizio.cimadamore@oracle.com">maurizio.cimadamore@oracle.com</a>&gt;</span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
this is a biggie changeset [1] for separating the definite assignment logic from the exception analysis logic in Flow.java. The two logic used to share the same visitor - moving forward it would be nice to split the two routines in two separate visitors as there are use cases in which we might only need to run one of the two (i.e. when inferring thrown types of a lambda).<br>


<br>
The inspiration for this work came from the BGGA prototype which featured a similar refactoring. I have added my own little twist to the code - for example, both visitors are nested classes inside Flow - they also share a common super class that contains shared routine for handling jumps.<br>


<br>
[1] - <a href="http://cr.openjdk.java.net/~mcimadamore/7151580.0/webrev/" target="_blank">http://cr.openjdk.java.net/~<u></u>mcimadamore/7151580.0/webrev/</a> &lt;<a href="http://cr.openjdk.java.net/%7Emcimadamore/7151580.0/webrev/" target="_blank">http://cr.openjdk.java.net/%<u></u>7Emcimadamore/7151580.0/<u></u>webrev/</a>&gt;<br>

<font color="#888888">
<br>
Maurizio<br>
<br>
</font></blockquote></div><br>