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