Merge pull request jruby#9190 from headius/implicit_var_fixes

Backport implicit local variable fixes from 10.1

Author: headius

core/src/main/java/org/jruby/parser/RubyParserBase.java

@@ -216,7 +216,7 @@ private ListNode makePreNumArgs(int paramCount) {
 
         for (int i = 1; i <= paramCount; i++) {
             RubySymbol name = symbolID(new ByteList(("_" + i).getBytes()));
-            list.add(new ArgumentNode(lexer.getRubySourceline(), name, getCurrentScope().addVariableThisScope(name.idString())));
+            list.add(new ArgumentNode(lexer.getRubySourceline(), name, getCurrentScope().addImplicitVariableThisScope(name.idString())));
         }
 
         return list;
@@ -2465,6 +2465,9 @@ protected Node it_id() {
     }
 
     protected void set_it_id(Node node) {
+        if (node != null) {
+            currentScope.markImplicitVariable(((DVarNode) node).getIndex());
+        }
         this.itId = node;
     }
 

core/src/main/java/org/jruby/parser/StaticScope.java

@@ -86,6 +86,8 @@ public class StaticScope implements Serializable, Cloneable {
     public static final int MAX_SPECIALIZED_SIZE = 50;
     private static final long serialVersionUID = 3423852552352498148L;
 
+    public static final int IMPLICIT = -2;
+
     // Next immediate scope.  Variable and constant scoping rules make use of this variable
     // in different ways.
     protected StaticScope enclosingScope;
@@ -100,6 +102,8 @@ public class StaticScope implements Serializable, Cloneable {
     // as key to Symbol table for actual encoded versions].
     private String[] variableNames;
 
+    private BitSet implicitVariables;
+
     private int variableNamesLength;
 
     // Arity of this scope if there is one
@@ -260,6 +264,36 @@ public int addVariableThisScope(String name) {
 
         if (slot >= 0) return slot;
 
+        // Clear constructor since we are adding a name
+        return addVariableName(name);
+    }
+
+    /**
+     * Add an implicit variable ("it", "_1") to this (current) scope unless it is already defined in the
+     * current scope. The variable will be marked as implicit to omit it from local variable lists and functions.
+     *
+     * @param name of new variable
+     * @return index of variable
+     */
+    public int addImplicitVariableThisScope(String name) {
+        int slot = exists(name);
+
+        if (slot >= 0) return slot;
+
+        slot = addVariableName(name);
+
+        markImplicitVariable(slot);
+
+        return slot;
+    }
+
+    public void markImplicitVariable(int slot) {
+        if (implicitVariables == null) implicitVariables = new BitSet();
+
+        implicitVariables.set(slot);
+    }
+
+    private int addVariableName(String name) {
         // Clear constructor since we are adding a name
         constructor = null;
 
@@ -283,13 +317,7 @@ public int addVariable(String name) {
         if (slot >= 0) return slot;
 
         // Clear constructor since we are adding a name
-        constructor = null;
-
-        // This is perhaps innefficient timewise?  Optimal spacewise
-        growVariableNames(name);
-
-        // Returns slot of variable
-        return variableNames.length - 1;
+        return addVariableName(name);
     }
 
     public String[] getVariables() {
@@ -396,6 +424,18 @@ public int exists(String name) {
         return findVariableName(name);
     }
 
+    /**
+     * Does the variable exist and not implicit?
+     *
+     * @param name of the variable to find
+     * @return index of variable; -1 if it does not exist; -2 if is implicit.
+     */
+    public int existsAndNotImplicit(String name) {
+        int slot = findVariableName(name);
+        if (slot >= 0 && isImplicitVariable(slot)) return IMPLICIT;
+        return slot;
+    }
+
     private int findVariableName(String name) {
         for (int i = 0; i < variableNames.length; i++) {
             if (name.equals(variableNames[i])) return i;
@@ -414,6 +454,24 @@ public int isDefined(String name) {
         return isDefined(name, 0);
     }
 
+    /**
+     * Is this name visible to the current scope and not an implicit variable ("it", "_1", etc).
+     *
+     * @param name to be looked for
+     * @return -1 if it is not defined; -2 if it is implicit; or a location where the left-most 16 bits of number of scopes down it is and the
+     * right-most 16 bits represents its index in that scope.
+     */
+    public int isDefinedNotImplicit(String name) {
+        return isDefinedNotImplicit(name, 0);
+    }
+
+    /**
+     * @return whether the given slot contains an implicit variable ("it", "_1", etc).
+     */
+    public boolean isImplicitVariable(int slot) {
+        return implicitVariables != null && implicitVariables.get(slot);
+    }
+
     /**
      * Make a DASgn or LocalAsgn node based on scope logic
      *
@@ -477,6 +535,21 @@ public <T> T collectVariables(IntFunction<T> collectionFactory, BiConsumer<T, St
 
     }
 
+    /**
+     * Populate a deduplicated collection of variable names in scope using the given functions.
+     *
+     * This may include variables that are not strictly Ruby local variable names, so the consumer should validate
+     * names as appropriate.
+     *
+     * @param collectionFactory used to construct the collection
+     * @param collectionPopulator used to pass values into the collection
+     * @param <T> resulting collection type
+     * @return populated collection
+     */
+    public <T> T collectAllVariables(ThreadContext context, BiFunction<ThreadContext, Integer, T> collectionFactory, BiConsumer<T, String> collectionPopulator) {
+        return collectVariables(context, collectionFactory, collectionPopulator, null);
+    }
+
     /**
      * Populate a deduplicated collection of variable names in scope using the given functions.
      *
@@ -489,27 +562,64 @@ public <T> T collectVariables(IntFunction<T> collectionFactory, BiConsumer<T, St
      * @return populated collection
      */
     public <T> T collectVariables(ThreadContext context, BiFunction<ThreadContext, Integer, T> collectionFactory, BiConsumer<T, String> collectionPopulator) {
+        return collectVariables(context, collectionFactory, collectionPopulator, false);
+    }
+
+    /**
+     * Populate a deduplicated collection of implicit variable names("it", "_1", etc) in scope using the given functions.
+     *
+     * This may include variables that are not strictly Ruby local variable names, so the consumer should validate
+     * names as appropriate.
+     *
+     * @param collectionFactory used to construct the collection
+     * @param collectionPopulator used to pass values into the collection
+     * @param <T> resulting collection type
+     * @return populated collection
+     */
+    public <T> T collectImplicitVariables(ThreadContext context, BiFunction<ThreadContext, Integer, T> collectionFactory, BiConsumer<T, String> collectionPopulator) {
+        return collectVariables(context, collectionFactory, collectionPopulator, true);
+    }
+
+    public <T> T collectVariables(ThreadContext context, BiFunction<ThreadContext, Integer, T> collectionFactory, BiConsumer<T, String> collectionPopulator, Boolean implicit) {
         StaticScope current = this;
 
         T collection = collectionFactory.apply(context, current.variableNamesLength);
 
         HashMap<String, Object> dedup = new HashMap<>();
 
         while (current.isBlockOrEval) {
-            for (String name : current.variableNames) {
-                dedup.computeIfAbsent(name, key -> {collectionPopulator.accept(collection, key); return key;});
-            }
+            addVariableNamesToCollection(current, collectionPopulator, dedup, collection, implicit);
             current = current.enclosingScope;
         }
 
         // once more for method scope
-        for (String name : current.variableNames) {
-            dedup.computeIfAbsent(name, key -> {collectionPopulator.accept(collection, key); return key;});
-        }
+        addVariableNamesToCollection(current, collectionPopulator, dedup, collection, implicit);
 
         return collection;
     }
 
+    private static <T> void addVariableNamesToCollection(StaticScope current, BiConsumer<T, String> collectionPopulator, HashMap<String, Object> dedup, T collection, Boolean implicit) {
+        BitSet implicitVariables = current.implicitVariables;
+
+        boolean hasImplicits = implicitVariables != null;
+        boolean filterImplicits = implicit != null;
+        if (filterImplicits && implicit && !hasImplicits) return;
+
+        int variableNamesLength = current.variableNamesLength;
+        String[] variableNames = current.variableNames;
+
+        for (int i = 0; i < variableNamesLength; i++) {
+            if (hasImplicits && implicitVariables.get(i)) {
+                if (filterImplicits && !implicit) continue;
+            } else if (filterImplicits && implicit) continue;
+
+            String name = variableNames[i];
+            dedup.computeIfAbsent(name, key -> {
+                collectionPopulator.accept(collection, key);
+                return key;});
+        }
+    }
+
     @Deprecated(since = "10.0.0.0")
     public RubyArray getLocalVariables(Ruby runtime) {
         return getLocalVariables(runtime.getCurrentContext());
@@ -522,13 +632,15 @@ public RubyArray getLocalVariables(Ruby runtime) {
      * @return populated RubyArray
      */
     public RubyArray getLocalVariables(ThreadContext context) {
-        return collectVariables(
+        return collectAllVariables(
                 context,
-                (ctxt, length) -> allocArray(ctxt, length),
-                (array, id) -> {
-                    RubySymbol symbol = Convert.asSymbol(context, id);
-                    if (symbol.validLocalVariableName()) array.append(context, symbol);
-                });
+                Create::allocArray,
+                (array, id) -> appendVariableIfValid(context, array, id));
+    }
+
+    private static void appendVariableIfValid(ThreadContext context, RubyArray<?> array, String id) {
+        RubySymbol symbol = Convert.asSymbol(context, id);
+        if (symbol.validLocalVariableName()) array.append(context, symbol);
     }
 
     public int isDefined(String name, int depth) {
@@ -542,6 +654,19 @@ public int isDefined(String name, int depth) {
         }
     }
 
+    public int isDefinedNotImplicit(String name, int depth) {
+        int slot = existsAndNotImplicit(name);
+        if (slot == IMPLICIT) return slot;
+
+        if (isBlockOrEval) {
+            if (slot >= 0) return (depth << 16) | slot;
+
+            return enclosingScope.isDefinedNotImplicit(name, depth + 1);
+        } else {
+            return (depth << 16) | slot;
+        }
+    }
+
     public AssignableNode addAssign(int line, RubySymbol symbolID, Node value) {
         int slot = addVariable(symbolID.idString());
         // No bit math to store level since we know level is zero for this case