8159977: typeof operator does not see lexical bindings declared in other scripts
authorhannesw
Wed, 22 Jun 2016 16:30:41 +0200
changeset 39164 d40cf0f38270
parent 39107 2a5697a98620
child 39165 f0000753cecd
8159977: typeof operator does not see lexical bindings declared in other scripts Reviewed-by: sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptRuntime.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java
nashorn/test/src/jdk/nashorn/internal/runtime/test/LexicalBindingTest.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java	Wed Jul 05 21:52:00 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/Global.java	Wed Jun 22 16:30:41 2016 +0200
@@ -2474,14 +2474,14 @@
     }
 
     @Override
-    protected FindProperty findProperty(final Object key, final boolean deep, final ScriptObject start) {
-        if (lexicalScope != null && start != this && start.isScope()) {
+    protected FindProperty findProperty(final Object key, final boolean deep, boolean isScope, final ScriptObject start) {
+        if (lexicalScope != null && isScope) {
             final FindProperty find = lexicalScope.findProperty(key, false);
             if (find != null) {
                 return find;
             }
         }
-        return super.findProperty(key, deep, start);
+        return super.findProperty(key, deep, isScope, start);
     }
 
     @Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Wed Jul 05 21:52:00 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Wed Jun 22 16:30:41 2016 +0200
@@ -775,7 +775,7 @@
      * @return FindPropertyData or null if not found.
      */
     public final FindProperty findProperty(final Object key, final boolean deep) {
-        return findProperty(key, deep, this);
+        return findProperty(key, deep, false, this);
     }
 
     /**
@@ -791,12 +791,12 @@
      * @see jdk.nashorn.internal.objects.NativeArray
      *
      * @param key  Property key.
-     * @param deep Whether the search should look up proto chain.
+     * @param deep true if the search should look up proto chain
+     * @param isScope true if this is a scope access
      * @param start the object on which the lookup was originally initiated
-     *
      * @return FindPropertyData or null if not found.
      */
-    protected FindProperty findProperty(final Object key, final boolean deep, final ScriptObject start) {
+    protected FindProperty findProperty(final Object key, final boolean deep, boolean isScope, final ScriptObject start) {
 
         final PropertyMap selfMap  = getMap();
         final Property    property = selfMap.findProperty(key);
@@ -807,7 +807,7 @@
 
         if (deep) {
             final ScriptObject myProto = getProto();
-            final FindProperty find = myProto == null ? null : myProto.findProperty(key, true, start);
+            final FindProperty find = myProto == null ? null : myProto.findProperty(key, true, isScope, start);
             // checkSharedProtoMap must be invoked after myProto.checkSharedProtoMap to propagate
             // shared proto invalidation up the prototype chain. It also must be invoked when prototype is null.
             checkSharedProtoMap();
@@ -1977,7 +1977,7 @@
             return findMegaMorphicGetMethod(desc, name, operation == StandardOperation.GET_METHOD);
         }
 
-        final FindProperty find = findProperty(name, true);
+        final FindProperty find = findProperty(name, true, NashornCallSiteDescriptor.isScope(desc), this);
         MethodHandle mh;
 
         if (find == null) {
@@ -2035,7 +2035,7 @@
     }
 
     private static GuardedInvocation findMegaMorphicGetMethod(final CallSiteDescriptor desc, final String name, final boolean isMethod) {
-        Context.getContextTrusted().getLogger(ObjectClassGenerator.class).warning("Megamorphic getter: " + desc + " " + name + " " +isMethod);
+        Context.getContextTrusted().getLogger(ObjectClassGenerator.class).warning("Megamorphic getter: ", desc, " ", name + " ", isMethod);
         final MethodHandle invoker = MH.insertArguments(MEGAMORPHIC_GET, 1, name, isMethod, NashornCallSiteDescriptor.isScope(desc));
         final MethodHandle guard   = getScriptObjectGuard(desc.getMethodType(), true);
         return new GuardedInvocation(invoker, guard);
@@ -2043,7 +2043,7 @@
 
     @SuppressWarnings("unused")
     private Object megamorphicGet(final String key, final boolean isMethod, final boolean isScope) {
-        final FindProperty find = findProperty(key, true);
+        final FindProperty find = findProperty(key, true, isScope, this);
         if (find != null) {
             return find.getObjectValue();
         }
@@ -2181,7 +2181,7 @@
          *
          * toString = function() { print("global toString"); } // don't affect Object.prototype.toString
          */
-        FindProperty find = findProperty(name, true, this);
+        FindProperty find = findProperty(name, true, NashornCallSiteDescriptor.isScope(desc), this);
 
         // If it's not a scope search, then we don't want any inherited properties except those with user defined accessors.
         if (find != null && find.isInherited() && !find.getProperty().isAccessorProperty()) {
@@ -2258,6 +2258,7 @@
     }
 
     private GuardedInvocation findMegaMorphicSetMethod(final CallSiteDescriptor desc, final String name) {
+        Context.getContextTrusted().getLogger(ObjectClassGenerator.class).warning("Megamorphic setter: ", desc, " ", name);
         final MethodType        type = desc.getMethodType().insertParameterTypes(1, Object.class);
         //never bother with ClassCastExceptionGuard for megamorphic callsites
         final GuardedInvocation inv = findSetIndexMethod(getClass(), desc, false, type);
@@ -2734,7 +2735,7 @@
         if (isValidArrayIndex(index)) {
             for (ScriptObject object = this; ; ) {
                 if (object.getMap().containsArrayKeys()) {
-                    final FindProperty find = object.findProperty(key, false, this);
+                    final FindProperty find = object.findProperty(key, false);
 
                     if (find != null) {
                         return getIntValue(find, programPoint);
@@ -2805,7 +2806,7 @@
         if (isValidArrayIndex(index)) {
             for (ScriptObject object = this; ; ) {
                 if (object.getMap().containsArrayKeys()) {
-                    final FindProperty find = object.findProperty(key, false, this);
+                    final FindProperty find = object.findProperty(key, false);
                     if (find != null) {
                         return getDoubleValue(find, programPoint);
                     }
@@ -2875,7 +2876,7 @@
         if (isValidArrayIndex(index)) {
             for (ScriptObject object = this; ; ) {
                 if (object.getMap().containsArrayKeys()) {
-                    final FindProperty find = object.findProperty(key, false, this);
+                    final FindProperty find = object.findProperty(key, false);
 
                     if (find != null) {
                         return find.getObjectValue();
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptRuntime.java	Wed Jul 05 21:52:00 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptRuntime.java	Wed Jun 22 16:30:41 2016 +0200
@@ -704,7 +704,17 @@
 
         if (property != null) {
             if (obj instanceof ScriptObject) {
-                obj = ((ScriptObject)obj).get(property);
+                // this is a scope identifier
+                assert property instanceof String;
+                final ScriptObject sobj = (ScriptObject) obj;
+
+                final FindProperty find = sobj.findProperty(property, true, true, sobj);
+                if (find != null) {
+                    obj = find.getObjectValue();
+                } else {
+                    obj = sobj.invokeNoSuchProperty(property, false, UnwarrantedOptimismException.INVALID_PROGRAM_POINT);
+                }
+
                 if(Global.isLocationPropertyPlaceholder(obj)) {
                     if(CompilerConstants.__LINE__.name().equals(property)) {
                         obj = 0;
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java	Wed Jul 05 21:52:00 2017 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/WithObject.java	Wed Jun 22 16:30:41 2016 +0200
@@ -193,20 +193,20 @@
      *
      * @param key  Property key.
      * @param deep Whether the search should look up proto chain.
+     * @param isScope true if is this a scope access
      * @param start the object on which the lookup was originally initiated
-     *
      * @return FindPropertyData or null if not found.
      */
     @Override
-    protected FindProperty findProperty(final Object key, final boolean deep, final ScriptObject start) {
+    protected FindProperty findProperty(final Object key, final boolean deep, boolean isScope, final ScriptObject start) {
         // We call findProperty on 'expression' with 'expression' itself as start parameter.
         // This way in ScriptObject.setObject we can tell the property is from a 'with' expression
         // (as opposed from another non-scope object in the proto chain such as Object.prototype).
-        final FindProperty exprProperty = expression.findProperty(key, true, expression);
+        final FindProperty exprProperty = expression.findProperty(key, true, false, expression);
         if (exprProperty != null) {
              return exprProperty;
         }
-        return super.findProperty(key, deep, start);
+        return super.findProperty(key, deep, isScope, start);
     }
 
     @Override
--- a/nashorn/test/src/jdk/nashorn/internal/runtime/test/LexicalBindingTest.java	Wed Jul 05 21:52:00 2017 +0200
+++ b/nashorn/test/src/jdk/nashorn/internal/runtime/test/LexicalBindingTest.java	Wed Jun 22 16:30:41 2016 +0200
@@ -46,8 +46,8 @@
 public class LexicalBindingTest {
 
     final static String LANGUAGE_ES6 = "--language=es6";
-    final static int NUMBER_OF_CONTEXTS = 20;
-    final static int MEGAMORPHIC_LOOP_COUNT = 20;
+    final static int NUMBER_OF_CONTEXTS = 40;
+    final static int MEGAMORPHIC_LOOP_COUNT = 40;
 
     /**
      * Test access to global var-declared variables for shared script classes with multiple globals.
@@ -57,19 +57,21 @@
         final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
         final ScriptEngine e = factory.getScriptEngine();
         final ScriptContext[] contexts = new ScriptContext[NUMBER_OF_CONTEXTS];
-        final String sharedScript = "foo";
+        final String sharedScript1 = "foo";
+        final String sharedScript2 = "bar = foo; bar";
 
 
         for (int i = 0; i < NUMBER_OF_CONTEXTS; i++) {
             final ScriptContext context = contexts[i] = new SimpleScriptContext();
             final Bindings b = e.createBindings();
             context.setBindings(b, ScriptContext.ENGINE_SCOPE);
-            assertEquals(e.eval("var foo = '" + i + "';", context), null);
+            assertEquals(e.eval("var foo = '" + i + "'; var bar;", context), null);
         }
 
         for (int i = 0; i < NUMBER_OF_CONTEXTS; i++) {
             final ScriptContext context = contexts[i];
-            assertEquals(e.eval(sharedScript, context), String.valueOf(i));
+            assertEquals(e.eval(sharedScript1, context), String.valueOf(i));
+            assertEquals(e.eval(sharedScript2, context), String.valueOf(i));
         }
     }
 
@@ -81,19 +83,21 @@
         final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
         final ScriptEngine e = factory.getScriptEngine(LANGUAGE_ES6);
         final ScriptContext[] contexts = new ScriptContext[NUMBER_OF_CONTEXTS];
-        final String sharedScript = "foo";
+        final String sharedScript1 = "foo";
+        final String sharedScript2 = "bar = foo; bar";
 
 
         for (int i = 0; i < NUMBER_OF_CONTEXTS; i++) {
             final ScriptContext context = contexts[i] = new SimpleScriptContext();
             final Bindings b = e.createBindings();
             context.setBindings(b, ScriptContext.ENGINE_SCOPE);
-            assertEquals(e.eval("let foo = '" + i + "';", context), null);
+            assertEquals(e.eval("let foo = '" + i + "'; let bar; ", context), null);
         }
 
         for (int i = 0; i < NUMBER_OF_CONTEXTS; i++) {
             final ScriptContext context = contexts[i];
-            assertEquals(e.eval(sharedScript, context), String.valueOf(i));
+            assertEquals(e.eval(sharedScript1, context), String.valueOf(i));
+            assertEquals(e.eval(sharedScript2, context), String.valueOf(i));
         }
     }
 
@@ -182,6 +186,27 @@
         assertEquals(e.eval(sharedScript, newCtxt), "newer context");
     }
 
+    /**
+     * Make sure lexically defined variables are accessible in other scripts.
+     */
+    @Test
+    public void lexicalScopeTest() throws ScriptException {
+        final NashornScriptEngineFactory factory = new NashornScriptEngineFactory();
+        final ScriptEngine e = factory.getScriptEngine(LANGUAGE_ES6);
+
+        e.eval("let x; const y = 'world';");
+
+        assertEquals(e.eval("x = 'hello'"), "hello");
+        assertEquals(e.eval("typeof x"), "string");
+        assertEquals(e.eval("typeof y"), "string");
+        assertEquals(e.eval("x"), "hello");
+        assertEquals(e.eval("y"), "world");
+        assertEquals(e.eval("typeof this.x"), "undefined");
+        assertEquals(e.eval("typeof this.y"), "undefined");
+        assertEquals(e.eval("this.x"), null);
+        assertEquals(e.eval("this.y"), null);
+    }
+
     private static class ScriptRunner implements Runnable {
 
         final ScriptEngine engine;