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

Attila Szegedi attila.szegedi at oracle.com
Mon Sep 14 10:19:19 UTC 2015


+1

> On Sep 14, 2015, at 11:23 AM, Sundararajan Athijegannathan <sundararajan.athijegannathan at 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.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
> 



More information about the nashorn-dev mailing list