[8u] RFR 8055917: jdk.nashorn.internal., , codegen, , .CompilationPhase$N should be renamed to proper classes

[Hannes Wallnoefer]

+1

I like the new, simpler code!

Am 2015-09-14 um 11:23 schrieb Sundararajan Athijegannathan:
> 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.athijegannathan at 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
>