+1 On Mar 9, 2015, at 2:23 PM, A. Sundararajan <sundararajan.athijegannat...@oracle.com> wrote:
> Thanks for the review. Please review the updated webrev: > > http://cr.openjdk.java.net/~sundar/8074671/webrev.01/ > > Changes: > > * Fixed JSONWriter.java "should not reach here" string. > * Fixed typo in withcheck.js > * moved synthetic flag to Block.java - added a test case to > test/script/nosecurity/parservisitor.js test > * changed stream().forEach() as forEach() as Remi suggested > (SimpleTreeVisitorES5_1.java) > > Thanks, > -Sundar > > On Monday 09 March 2015 01:57 PM, Attila Szegedi wrote: >> Amazing work Sundar. I can see this being a very strong point for Nashorn >> for people writing tools that work with JS source files. >> >> Two typos: >> - JSONWriter: assert "should reach here" should read "should not reach >> here" >> - withcheck.js: typo: "diganostic" >> >> I'm also very happy that we have synthetic marker on BlockStatement. That's >> actually something that we could now use for more precise let/const scoping. >> I think though that we should add the flag on Block, not on BlockStatement. >> Block already has a flags bitfield, BTW, but that's not the reason. The >> reason is that we're historically introducing synthetic blocks elsewhere >> too. "if(...) stmt;" becomes "if(...) { stmt; }" internally as our AST is >> structured so that IfNode, ForNode, WhileNode, CaseNode etc. do not hold >> Statement nodes as their bodies, but Block nodes instead. This is strictly >> speaking incorrect mapping of the ES grammar, but I guess that for now we're >> stuck with it. >> >> Those blocks are synthetic too, and we aren't distinguishing them on the AST >> level from explicit blocks present in the source code. It isn't much of a >> trouble for contexts where only a single statement can be specified without >> a block (which is most contexts in JS) but it can be a trouble for >> switch/case, as "case x: stmt1; stmt2; case y: ..." and "case x: { stmt1; >> stmt2; } case y: ..." have differing semantics in ES6 if stmt1 is a >> let/const. So I think that if we're finally adding a "synthetic" flag (as we >> should!) to blocks, it should be marked on Block, not BlockStatement. >> >> Attila. >> >> On Mar 9, 2015, at 7:02 AM, A. Sundararajan >> <sundararajan.athijegannat...@oracle.com> wrote: >> >>> Please review "Nashorn Parser API". Introduces jdk.nashorn.api.tree package >>> to support parser API for Nashorn. >>> >>> JEP: http://openjdk.java.net/jeps/236 >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8074671 >>> Webrev: http://cr.openjdk.java.net/~sundar/8074671/ >>> >>> Simple examples "evalcheck.js" and "withcheck.js" under samples directory >>> added. >>> >>> Thanks to Joe Darcy who helped me with extensive specification review. >>> >>> Thanks, >>> -Sundar >
