Silence named capture conflict warning if var is just another cap.

Fixes jruby#3865 by implementing a partial fix for https://bugs.ruby-lang.org/issues/12359
enebo · May 10, 2016 · 3ebc198 · 3ebc198
1 parent 9d86769
commit 3ebc198
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 4 deletions.
diff --git a/core/src/main/java/org/jruby/parser/ParserSupport.java b/core/src/main/java/org/jruby/parser/ParserSupport.java
@@ -1303,14 +1303,16 @@ private List<Integer> allocateNamedLocals(RegexpNode regexpNode) {
 
         for (int i = 0; i < length; i++) {
             // TODO: Pass by non-local-varnamed things but make sure consistent with list we get from regexp
-
             if (RubyLexer.getKeyword(names[i]) == null && !Character.isUpperCase(names[i].charAt(0))) {
                 int slot = scope.isDefined(names[i]);
                 if (slot >= 0) {
-                    if (warnings.isVerbose()) warn(ID.AMBIGUOUS_ARGUMENT, getPosition(regexpNode), "named capture conflicts a local variable - " + names[i]);
+                    // If verbose and the variable is not just another named capture, warn
+                    if (warnings.isVerbose() && !scope.isNamedCapture(slot)) {
+                        warn(ID.AMBIGUOUS_ARGUMENT, getPosition(regexpNode), "named capture conflicts a local variable - " + names[i]);
+                    }
                     locals.add(slot);
                 } else {
-                    locals.add(getCurrentScope().addVariableThisScope(names[i]));
+                    locals.add(getCurrentScope().addNamedCaptureVariable(names[i]));
                 }
             }
         }

diff --git a/core/src/main/java/org/jruby/parser/StaticScope.java b/core/src/main/java/org/jruby/parser/StaticScope.java
@@ -76,6 +76,9 @@ public class StaticScope implements Serializable {
     // Our name holder (offsets are assigned as variables are added)
     private String[] variableNames;
 
+    // A list of booleans indicating which variables are named captures from regexp
+    private boolean[] namedCaptures;
+
     // Arity of this scope if there is one
     private Signature signature;
 
@@ -185,7 +188,7 @@ private static boolean namesAreInterned(String[] names) {
      * current scope.
      *
      * @param name of new variable
-     * @return index+depth merged location of scope
+     * @return index of variable
      */
     public int addVariableThisScope(String name) {
         // Ignore duplicate "_" args in blocks
@@ -206,6 +209,20 @@ public int addVariableThisScope(String name) {
         return variableNames.length - 1;
     }
 
+    /**
+     * Add a new named capture variable to this (current) scope.
+     *
+     * @param name name of variable.
+     * @return index of variable
+     */
+    public int addNamedCaptureVariable(String name) {
+        int index = addVariableThisScope(name);
+
+        growNamedCaptures(index);
+
+        return index;
+    }
+
     /**
      * Add a new variable to this (current) scope unless it is already defined in any
      * reachable scope.
@@ -509,6 +526,24 @@ private void growVariableNames(String name) {
         variableNames[variableNames.length - 1] = name;
     }
 
+    private void growNamedCaptures(int index) {
+        boolean[] namedCaptures = this.namedCaptures;
+        boolean[] newNamedCaptures;
+        if (namedCaptures != null) {
+            newNamedCaptures = new boolean[Math.max(index + 1, namedCaptures.length)];
+            System.arraycopy(namedCaptures, 0, newNamedCaptures, 0, namedCaptures.length);
+        } else {
+            newNamedCaptures = new boolean[index + 1];
+        }
+        newNamedCaptures[index] = true;
+        this.namedCaptures = newNamedCaptures;
+    }
+
+    public boolean isNamedCapture(int index) {
+        boolean[] namedCaptures = this.namedCaptures;
+        return namedCaptures != null && index < namedCaptures.length && namedCaptures[index];
+    }
+
     @Override
     public String toString() {
         // FIXME: Do we need to persist cref as well?