+1
> On Sep 14, 2015, at 11:23 AM, Sundararajan Athijegannathan
> <sundararajan.athijegannat...@oracle.com> wrote:
>
> Updated webrev as per the suggestions:
> http://cr.openjdk.java.net/~sundar/8055917/webrev.01/
>
> * CompilationPhase is an abstract class (not an enum).
> * Nested subclasses are private and final - static final field for each
> subclass instance
> * All subclasses follow ABCPhase name pattern (can't use ApplySpecialization,
> CacheAst etc. - as that would clash with class doing actual transformation)
> * Removed CompilationState.
>
> Thanks,
> -Sundar
>
> On 9/14/2015 1:27 PM, Attila Szegedi wrote:
>> I’d achieve the same effect by changing CompilationPhase to be a regular
>> class instead of an enum, and creating named inner classes.
>>
>> While we’re at it, let’s also rename “BuiltinTransform” to something closer
>> to “ApplySpecialization”.
>>
>> For bonus points, you might consider removing CompilationState altogether.
>> CompilationState serves no functional purpose just to ensure that a phase
>> only runs when it's preceding states did. In fact, some compilation phases
>> have been added since that don’t even bother updating CompilationState
>> anymore, so it’s effectively abandoned.
>>
>> Attila.
>>
>> [9/14/15, 9:39:54 AM] Attila Szegedi: but we guarantee that with the
>> construction of phases in Compiler.java anyway
>>> On Sep 14, 2015, at 9:04 AM, Sundararajan Athijegannathan
>>> <sundararajan.athijegannat...@oracle.com> wrote:
>>>
>>> Please review http://cr.openjdk.java.net/~sundar/8055917/ for
>>> https://bugs.openjdk.java.net/browse/JDK-8055917
>>>
>>> PS. Piggybacking few cleanups in samples and also adding 2 new samples.
>>>
>>> Thanks,
>>> -Sundar
>
