Review request for JDK-8006529

A. Sundararajan sundararajan.athijegannathan at oracle.com
Thu Jan 31 03:46:49 PST 2013


As Attila said, it is test-only code under test/src dir. Only issue is 
that the reflective and scripts dependencies on nashorn internal 
structures won't be caught by compile time.  May be a package-private 
accessor and test code could use the same package? Again, only to help 
finding dependencies by refactoring tools (if possible). In any case, 
test(s) will fail if anyone makes incompatible change and so developer 
making such change will have to sync test cases - which is okay.

-Sundar

On Thursday 31 January 2013 04:55 PM, Attila Szegedi wrote:
> On Jan 30, 2013, at 5:20 PM, Jim Laskey (Oracle) <james.laskey at oracle.com> wrote:
>
>> FinalizeTypes.java
>>
>> 549     private static void updateSymbols(final Block block) {
>> 550         if (block instanceof FunctionNode && block.getFunction() == block) {
>>
>> Why not a visitor (instead of instanceof test)?
>> and Shouldn't it be?
>>
>> 549     private static void updateSymbols(final Block block) {
>> 550         if (block instanceof FunctionNode) {
>> 551              assert block.getFunction() == block;
>>
> You're right - I should move this into enter(FunctionNode).
>
>> I have to say jdk.nashorn.internal.codegen.CompilerAccess creeps me out a bit.  Since it's for testing only, it should not get into fcs build.
> Yup, it's very deliberately put into test directory. OTOH, this is something that anyone can do in absence of a security manager anyway (and can't do in presence of one).
>
> Alternatively, we can add a "public FunctionNode getRootFunctionNode()" to compiler - not sure if we want to expose it though (although not sure that we don't want to either - if you or Sundar have an opinion as to the preferred approach, I'm happy to hear it).
>
>> +1 otherwise.
> Thanks,
>    Attila.
>
>>
>> On 2013-01-30, at 11:53 AM, Attila Szegedi <attila.szegedi at oracle.com> wrote:
>>
>>> Please review JDK-8006529 at http://cr.openjdk.java.net/~attila/8006529/webrev.00
>>>
>>> Thanks,
>>> Attila.



More information about the nashorn-dev mailing list