Looks great.<br><br><div class="gmail_quote">On Mon, Mar 12, 2012 at 5:38 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">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    Thanks for the comments - updated webrev at the following URL:<br>
    <br>
    
    <a href="http://cr.openjdk.java.net/%7Emcimadamore/7151580.1/" target="_blank">http://cr.openjdk.java.net/~mcimadamore/7151580.1/</a><br>
    <br>
    *) I added comments to all visitor classes (including abstract base
    class) in Flow.java<br>
    *) I refactored the statement:<br>
    <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>
    Outside the enclosing if statement as suggested.<br><font color="#888888">
    <br>
    Maurizio</font><div><div></div><div class="h5"><br>
    <br>
    On 09/03/12 21:56, Neal Gafter wrote:
    <blockquote type="cite"><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" target="_blank">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/%7Emcimadamore/7151580.0/webrev/" target="_blank">http://cr.openjdk.java.net/~mcimadamore/7151580.0/webrev/</a>
          &lt;<a href="http://cr.openjdk.java.net/%7Emcimadamore/7151580.0/webrev/" target="_blank">http://cr.openjdk.java.net/%7Emcimadamore/7151580.0/webrev/</a>&gt;<br>
          <font color="#888888">
            <br>
            Maurizio<br>
            <br>
          </font></blockquote>
      </div>
      <br>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br>