Code review request: 7151580 - Separate DA/DU logic from exception checking logic in Flow.java

[Neal Gafter]

Looks great.

On Mon, Mar 12, 2012 at 5:38 AM, Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:

>  Thanks for the comments - updated webrev at the following URL:
>
> http://cr.openjdk.java.net/~mcimadamore/7151580.1/
>
> *) I added comments to all visitor classes (including abstract base class)
> in Flow.java
> *) I refactored the statement:
>
> tree.finallyCanCompleteNormally = alive;
>
>
>  Outside the enclosing if statement as suggested.
>
> Maurizio
>
>
> On 09/03/12 21:56, Neal Gafter wrote:
>
> Maurizio-
>
>  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't clear that would be a win here.
>
>  It would be helpful to have comments at the top of FlowAnalyzer and
> AssignAnalyzer explaining what they do (i.e. the division of labor).
>
>  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:
>
>  tree.finallyCanCompleteNormally = alive;
>
>
>  Cheers,
> Neal
>
> On Fri, Mar 9, 2012 at 3:00 AM, Maurizio Cimadamore <
> maurizio.cimadamore at oracle.com> wrote:
>
>> Hi,
>> 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).
>>
>> 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.
>>
>> [1] - http://cr.openjdk.java.net/~mcimadamore/7151580.0/webrev/ <
>> http://cr.openjdk.java.net/%7Emcimadamore/7151580.0/webrev/>
>>
>> Maurizio
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20120315/403853ae/attachment.html