Permalink
Browse files

Avoid using backref within native logic.

Some native-implemented methods like String#to_r and to_c used
Regexp logic to break apart the string. Because they called
through the same paths that Ruby code calls through, they were
forced to prepare for and use backref ($~) to hold the MatchData
object. My changes provide a backref-free path in RubyRegexp and
modifies our native logic to use that path. This eliminates the
need for a DynamicScope to be created and pushed along those paths
and should provide a path forward for eliminating scopes from Ruby
code that does not directly reference $~ and friends.
  • Loading branch information...
1 parent 0e5a983 commit d00e21dfd6291f1a205686458f9502a1e8fc86e2 @headius headius committed Apr 15, 2013
Showing with 117 additions and 100 deletions.
  1. +19 −21 src/org/jruby/RubyComplex.java
  2. +7 −11 src/org/jruby/RubyRational.java
  3. +61 −19 src/org/jruby/RubyRegexp.java
  4. +30 −49 src/org/jruby/RubyString.java
@@ -61,6 +61,7 @@
import org.jruby.anno.JRubyClass;
import org.jruby.anno.JRubyMethod;
import org.jruby.runtime.Arity;
+import org.jruby.runtime.Block;
import org.jruby.runtime.ClassIndex;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ObjectAllocator;
@@ -445,15 +446,9 @@ public static IRubyObject convert(ThreadContext context, IRubyObject recv, IRuby
}
private static IRubyObject convertCommon(ThreadContext context, IRubyObject recv, IRubyObject a1, IRubyObject a2) {
- DynamicScope scope = context.getCurrentScope();
- IRubyObject backref = scope.getBackRef(context.runtime);
- if (backref instanceof RubyMatchData) ((RubyMatchData)backref).use();
-
if (a1 instanceof RubyString) a1 = str_to_c_strict(context, a1);
if (a2 instanceof RubyString) a2 = str_to_c_strict(context, a2);
- scope.setBackRef(backref);
-
if (a1 instanceof RubyComplex) {
RubyComplex a1Complex = (RubyComplex)a1;
if (k_exact_p(a1Complex.image) && f_zero_p(context, a1Complex.image)) {
@@ -970,43 +965,46 @@ static RubyArray str_to_c_internal(ThreadContext context, IRubyObject recv) {
IRubyObject sr, si, re;
sr = si = re = runtime.getNil();
boolean po = false;
- IRubyObject m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat0).callMethod(context, "match", s);
+ IRubyObject m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat0).match_m19(context, s, false, Block.NULL_BLOCK);
if (!m.isNil()) {
- sr = m.callMethod(context, "[]", RubyFixnum.one(runtime));
- si = m.callMethod(context, "[]", RubyFixnum.two(runtime));
- re = m.callMethod(context, "post_match");
+ RubyMatchData match = (RubyMatchData)m;
+ sr = match.op_aref19(RubyFixnum.one(runtime));
+ si = match.op_aref19(RubyFixnum.two(runtime));
+ re = match.post_match(context);
po = true;
}
if (m.isNil()) {
- m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat1).callMethod(context, "match", s);
+ m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat1).match_m19(context, s, false, Block.NULL_BLOCK);
if (!m.isNil()) {
+ RubyMatchData match = (RubyMatchData)m;
sr = runtime.getNil();
- si = m.callMethod(context, "[]", RubyFixnum.one(runtime));
+ si = match.op_aref19(RubyFixnum.one(runtime));
if (si.isNil()) si = runtime.newString();
- IRubyObject t = m.callMethod(context, "[]", RubyFixnum.two(runtime));
+ IRubyObject t = match.op_aref19(RubyFixnum.two(runtime));
if (t.isNil()) t = runtime.newString(new ByteList(new byte[]{'1'}));
si.convertToString().cat(t.convertToString().getByteList());
- re = m.callMethod(context, "post_match");
+ re = match.post_match(context);
po = false;
}
}
if (m.isNil()) {
- m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat2).callMethod(context, "match", s);
+ m = RubyRegexp.newDummyRegexp(runtime, Numeric.ComplexPatterns.comp_pat2).match_m19(context, s, false, Block.NULL_BLOCK);
if (m.isNil()) return runtime.newArray(runtime.getNil(), recv);
- sr = m.callMethod(context, "[]", RubyFixnum.one(runtime));
- if (m.callMethod(context, "[]", RubyFixnum.two(runtime)).isNil()) {
+ RubyMatchData match = (RubyMatchData)m;
+ sr = match.op_aref19(RubyFixnum.one(runtime));
+ if (match.op_aref19(RubyFixnum.two(runtime)).isNil()) {
si = runtime.getNil();
} else {
- si = m.callMethod(context, "[]", RubyFixnum.three(runtime));
- IRubyObject t = m.callMethod(context, "[]", RubyFixnum.four(runtime));
- if (t.isNil()) t = runtime.newString(new ByteList(new byte[]{'1'}));
+ si = match.op_aref19(RubyFixnum.three(runtime));
+ IRubyObject t = match.op_aref19(RubyFixnum.four(runtime));
+ if (t.isNil()) t = runtime.newString(RubyFixnum.SINGLE_CHAR_BYTELISTS19['1']);
si.convertToString().cat(t.convertToString().getByteList());
}
- re = m.callMethod(context, "post_match");
+ re = match.post_match(context);
po = false;
}
@@ -62,6 +62,7 @@
import org.jruby.anno.JRubyMethod;
import org.jruby.common.IRubyWarnings;
import org.jruby.runtime.Arity;
+import org.jruby.runtime.Block;
import org.jruby.runtime.ClassIndex;
import org.jruby.runtime.DynamicScope;
import org.jruby.runtime.ObjectAllocator;
@@ -382,10 +383,6 @@ private static IRubyObject convertCommon(ThreadContext context, IRubyObject recv
if (k_exact_p(a2Complex.getImage()) && f_zero_p(context, a2Complex.getImage())) a2 = a2Complex.getReal();
}
- DynamicScope scope = context.getCurrentScope();
- IRubyObject backref = scope.getBackRef(context.runtime);
- if (backref instanceof RubyMatchData) ((RubyMatchData)backref).use();
-
if (a1 instanceof RubyFloat) {
a1 = f_to_r(context, a1);
} else if (a1 instanceof RubyString) {
@@ -401,8 +398,6 @@ private static IRubyObject convertCommon(ThreadContext context, IRubyObject recv
} else if (a2 instanceof RubyString) {
a2 = str_to_r_strict(context, a2);
}
-
- scope.setBackRef(backref);
if (a1 instanceof RubyRational) {
if (a2.isNil() || (k_exact_p(a2) && f_one_p(context, a2))) return a1;
@@ -1028,13 +1023,14 @@ static RubyArray str_to_r_internal(ThreadContext context, IRubyObject recv) {
Ruby runtime = context.runtime;
if (bytes.getRealSize() == 0) return runtime.newArray(runtime.getNil(), recv);
- IRubyObject m = RubyRegexp.newDummyRegexp(runtime, Numeric.RationalPatterns.rat_pat).callMethod(context, "match", s);
+ IRubyObject m = RubyRegexp.newDummyRegexp(runtime, Numeric.RationalPatterns.rat_pat).match_m19(context, s, false, Block.NULL_BLOCK);
if (!m.isNil()) {
- IRubyObject si = m.callMethod(context, "[]", RubyFixnum.one(runtime));
- IRubyObject nu = m.callMethod(context, "[]", RubyFixnum.two(runtime));
- IRubyObject de = m.callMethod(context, "[]", RubyFixnum.three(runtime));
- IRubyObject re = m.callMethod(context, "post_match");
+ RubyMatchData match = (RubyMatchData)m;
+ IRubyObject si = match.op_aref19(RubyFixnum.one(runtime));
+ IRubyObject nu = match.op_aref19(RubyFixnum.two(runtime));
+ IRubyObject de = match.op_aref19(RubyFixnum.three(runtime));
+ IRubyObject re = match.post_match(context);
RubyArray a = nu.callMethod(context, "split", RubyRegexp.newDummyRegexp(runtime, Numeric.RationalPatterns.an_e_pat)).convertToArray();
IRubyObject ifp = a.eltInternal(0);
@@ -1512,43 +1512,63 @@ public IRubyObject match_m(ThreadContext context, IRubyObject str) {
@JRubyMethod(name = "match", reads = BACKREF, compat = CompatVersion.RUBY1_9)
public IRubyObject match_m19(ThreadContext context, IRubyObject str, Block block) {
- return match19Common(context, str, 0, block);
+ return match19Common(context, str, 0, true, block);
+ }
+
+ public IRubyObject match_m19(ThreadContext context, IRubyObject str, boolean useBackref, Block block) {
+ return match19Common(context, str, 0, useBackref, block);
}
@JRubyMethod(name = "match", reads = BACKREF, compat = CompatVersion.RUBY1_9)
public IRubyObject match_m19(ThreadContext context, IRubyObject str, IRubyObject pos, Block block) {
- return match19Common(context, str, RubyNumeric.num2int(pos), block);
+ return match19Common(context, str, RubyNumeric.num2int(pos), true, block);
+ }
+
+ public static class MatchDataHolder {
+ public RubyMatchData matchData;
}
- private IRubyObject match19Common(ThreadContext context, IRubyObject arg, int pos, Block block) {
- DynamicScope scope = context.getCurrentScope();
+ private IRubyObject match19Common(ThreadContext context, IRubyObject arg, int pos, boolean useBackref, Block block) {
+ DynamicScope scope = null;
+
+ if (useBackref) scope = context.getCurrentScope();
if (arg.isNil()) {
- scope.setBackRef(arg);
+ if (useBackref) scope.setBackRef(arg);
return arg;
}
Ruby runtime = context.runtime;
RubyString str = operandCheck(runtime, arg);
+ MatchDataHolder backrefHolder = useBackref ? null : new MatchDataHolder();
- if (matchPos(context, str, pos) < 0) {
- scope.setBackRef(runtime.getNil());
+ if (matchPos(context, str, pos, backrefHolder) < 0) {
+ if (useBackref) scope.setBackRef(runtime.getNil());
return runtime.getNil();
}
- IRubyObject backref = scope.getBackRef(runtime);
- ((RubyMatchData)backref).use();
+ IRubyObject backref = null;
+ if (useBackref) {
+ backref = scope.getBackRef(runtime);
+ ((RubyMatchData)backref).use();
+ } else {
+ backref = backrefHolder.matchData;
+ }
if (block.isGiven()) return block.yield(context, backref);
return backref;
}
private int matchPos(ThreadContext context, RubyString str, int pos) {
+ return matchPos(context, str, pos, null);
+ }
+
+ private int matchPos(ThreadContext context, RubyString str, int pos, MatchDataHolder backrefHolder) {
if (pos != 0) {
if (pos < 0) {
pos += str.strLength();
if (pos < 0) return pos;
}
pos = StringSupport.offset(str, pos);
}
- return search19(context, str, pos, false);
+ return search19(context, str, pos, false, backrefHolder);
}
/** rb_reg_search
@@ -1565,7 +1585,7 @@ public final int search(ThreadContext context, RubyString str, int pos, boolean
int result = matcher.search(begin + pos, begin + (reverse ? 0 : realSize), Option.NONE);
if (result >= 0) {
- updateBackRef(context, str, scope, matcher);
+ updateBackRef(context, str, scope, matcher, true);
return result;
}
}
@@ -1574,21 +1594,30 @@ public final int search(ThreadContext context, RubyString str, int pos, boolean
return -1;
}
- private RubyMatchData updateBackRef(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher) {
- RubyMatchData match = updateBackRef(context, str, scope, matcher, pattern);
+ private RubyMatchData updateBackRef(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher, boolean useBackref) {
+ RubyMatchData match = updateBackRef(context, str, scope, matcher, pattern, useBackref);
match.regexp = this;
match.infectBy(this);
return match;
}
static final RubyMatchData updateBackRef(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher, Regex pattern) {
+ return updateBackRef(context, str, scope, matcher, pattern, true);
+ }
+
+ static final RubyMatchData updateBackRef(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher, Regex pattern, boolean useBackref) {
Ruby runtime = context.runtime;
- IRubyObject backref = scope.getBackRef(runtime);
+ IRubyObject backref;
+ if (useBackref) {
+ backref = scope.getBackRef(runtime);
+ } else {
+ backref = context.nil;
+ }
final RubyMatchData match;
boolean setBackRef = false;
if (backref.isNil() || ((RubyMatchData)backref).used()) {
match = new RubyMatchData(runtime);
- setBackRef = true;
+ setBackRef = useBackref;
} else {
match = (RubyMatchData)backref;
match.setTaint(false);
@@ -1617,8 +1646,13 @@ static final RubyMatchData updateBackRef(ThreadContext context, RubyString str,
}
public final int search19(ThreadContext context, RubyString str, int pos, boolean reverse) {
+ return search19(context, str, pos, reverse, null);
+ }
+
+ public final int search19(ThreadContext context, RubyString str, int pos, boolean reverse, MatchDataHolder backrefHolder) {
check();
- DynamicScope scope = context.getCurrentScope();
+ DynamicScope scope = null;
+ if (backrefHolder == null) scope = context.getCurrentScope();
ByteList value = str.getByteList();
if (pos <= value.getRealSize() && pos >= 0) {
@@ -1628,17 +1662,25 @@ public final int search19(ThreadContext context, RubyString str, int pos, boolea
int result = matcher.search(begin + pos, begin + (reverse ? 0 : realSize), Option.NONE);
if (result >= 0) {
- updateBackRef(context, str, scope, matcher).charOffsetUpdated = false;;
+ RubyMatchData matchData = updateBackRef(context, str, scope, matcher, backrefHolder == null);
+ matchData.charOffsetUpdated = false;
+ if (backrefHolder != null) backrefHolder.matchData = matchData;
return result;
}
}
- scope.setBackRef(context.runtime.getNil());
+ if (backrefHolder == null) scope.setBackRef(context.runtime.getNil());
return -1;
}
static final RubyMatchData updateBackRef19(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher, Regex pattern) {
- RubyMatchData match = updateBackRef(context, str, scope, matcher, pattern);
+ RubyMatchData match = updateBackRef(context, str, scope, matcher, pattern, true);
+ match.charOffsetUpdated = false;
+ return match;
+ }
+
+ static final RubyMatchData updateBackRef19(ThreadContext context, RubyString str, DynamicScope scope, Matcher matcher, Regex pattern, boolean useBackref) {
+ RubyMatchData match = updateBackRef(context, str, scope, matcher, pattern, useBackref);
match.charOffsetUpdated = false;
return match;
}
Oops, something went wrong.

0 comments on commit d00e21d

Please sign in to comment.