Skip to content

Commit

Permalink
Patch up some regressions in StringIO/IO#gets stuff.
Browse files Browse the repository at this point in the history
* Only gets should set $_.
* Only StringIO#each/each_line complain about limit=0.
* Arity-split StringIO#each_line.
* Remove LAST_LINE frame field requirement from StringIO#each and
  #each_line.
* Misc tidying.
headius committed Aug 4, 2017
1 parent 4ca5a44 commit 74d0588
Showing 4 changed files with 78 additions and 56 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/RubyEnumerator.java
Original file line number Diff line number Diff line change
@@ -139,7 +139,7 @@ public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String
return new RubyEnumerator(runtime, runtime.getEnumerator(), object, runtime.fastNewSymbol(method), new IRubyObject[] {arg});
}

public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String method, IRubyObject[] args) {
public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String method, IRubyObject... args) {
return new RubyEnumerator(runtime, runtime.getEnumerator(), object, runtime.fastNewSymbol(method), args); // TODO: make sure it's really safe to not to copy it
}

6 changes: 5 additions & 1 deletion core/src/main/java/org/jruby/RubyIO.java
Original file line number Diff line number Diff line change
@@ -2268,7 +2268,11 @@ public IRubyObject gets(ThreadContext context, IRubyObject rs, IRubyObject limit
private static final Getline.Callback<RubyIO, IRubyObject> GETLINE = new Getline.Callback<RubyIO, IRubyObject>() {
@Override
public IRubyObject getline(ThreadContext context, RubyIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
return self.getlineInner(context, rs, limit, chomp, null);
IRubyObject result = self.getlineInner(context, rs, limit, chomp, null);

if (!result.isNil()) context.setLastLine(result);

return result;
}
};

70 changes: 54 additions & 16 deletions core/src/main/java/org/jruby/ext/stringio/StringIO.java
Original file line number Diff line number Diff line change
@@ -339,7 +339,7 @@ public IRubyObject each(ThreadContext context, IRubyObject arg0, IRubyObject arg
}

// MRI: strio_each
@JRubyMethod(name = "each", writes = FrameField.LASTLINE)
@JRubyMethod(name = "each")
public IRubyObject each(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each", Helpers.arrayOf(arg0, arg1, arg2));

@@ -363,19 +363,49 @@ public IRubyObject each(ThreadContext context, IRubyObject[] args, Block block)
}
}

private static boolean isLastArg0(final IRubyObject[] args) {
final int len = args.length;
return len > 0 &&
! args[len - 1].isNil() &&
args[len - 1].checkStringType19().isNil() &&
RubyNumeric.num2long( args[len - 1] ) == 0 ;
@JRubyMethod(name = "each_line")
public IRubyObject each_line(ThreadContext context, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line");

return each(context, block);
}

@JRubyMethod(name = "each_line")
public IRubyObject each_line(ThreadContext context, IRubyObject arg0, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0);

return each(context, arg0, block);
}

@JRubyMethod(name = "each_line")
public IRubyObject each_line(ThreadContext context, IRubyObject arg0, IRubyObject arg1, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0, arg1);

return each(context, arg0, arg1, block);
}

@JRubyMethod(name = "each_line")
public IRubyObject each_line(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0, arg1, arg2);

return each(context, arg0, arg1, arg2, block);
}

@JRubyMethod(name = "each_line", optional = 2, writes = FrameField.LASTLINE)
public IRubyObject each_line(ThreadContext context, IRubyObject[] args, Block block) {
if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", args);

return each(context, args, block);
switch (args.length) {
case 0:
return each_line(context, block);
case 1:
return each_line(context, args[0], block);
case 2:
return each_line(context, args[0], args[1], block);
case 3:
return each_line(context, args[0], args[1], args[2], block);
default:
Arity.raiseArgumentError(context, args.length, 0, 3);
throw new RuntimeException("BUG");
}
}

@JRubyMethod(name = "lines", optional = 2)
@@ -557,7 +587,15 @@ public IRubyObject gets(ThreadContext context, IRubyObject[] args) {
private static final Getline.Callback<StringIO, IRubyObject> GETLINE = new Getline.Callback<StringIO, IRubyObject>() {
@Override
public IRubyObject getline(ThreadContext context, StringIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
return self.getline(context, rs, limit, chomp);
if (limit == 0) {
return RubyString.newEmptyString(context.runtime);
}

IRubyObject result = self.getline(context, rs, limit, chomp);

context.setLastLine(result);

return result;
}
};

@@ -566,6 +604,10 @@ public IRubyObject getline(ThreadContext context, StringIO self, IRubyObject rs,
public StringIO getline(ThreadContext context, StringIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
IRubyObject line;

if (limit == 0) {
throw context.runtime.newArgumentError("invalid limit: 0 for each_line");
}

while (!(line = self.getline(context, rs, limit, chomp)).isNil()) {
block.yieldSpecific(context, line);
}
@@ -592,11 +634,7 @@ public RubyArray getline(ThreadContext context, StringIO self, IRubyObject rs, i
private IRubyObject getline(ThreadContext context, final IRubyObject rs, int limit, boolean chomp) {
Ruby runtime = context.runtime;

IRubyObject str = context.nil;

if (limit == 0) {
throw runtime.newArgumentError("invalid limit: 0 for each_line");
}
IRubyObject str;

checkReadable();

56 changes: 18 additions & 38 deletions core/src/main/java/org/jruby/util/io/Getline.java
Original file line number Diff line number Diff line change
@@ -54,7 +54,7 @@ public static <Self, Return extends IRubyObject> Return getlineCall(ThreadContex

boolean chomp = false;
long limit;
IRubyObject rs, opt, optArg = context.nil, sepArg = null, limArg = null;
IRubyObject opt, optArg = context.nil, sepArg = null, limArg = null;

switch (argc) {
case 1:
@@ -85,67 +85,47 @@ public static <Self, Return extends IRubyObject> Return getlineCall(ThreadContex
}
}

rs = prepareGetsSeparator(context, sepArg, limArg, enc_io);
limit = prepareGetsLimit(context, sepArg, limArg);

Return result = getline.getline(context, self, rs, (int) limit, chomp, block);

if (!result.isNil()) context.setLastLine(result);

return result;
}

// MRI: prepare_getline_args, separator logic
private static IRubyObject prepareGetsSeparator(ThreadContext context, IRubyObject arg0, IRubyObject arg1, Encoding enc_io) {
Ruby runtime = context.runtime;
IRubyObject rs = runtime.getRecordSeparatorVar().get();
if (arg0 != null && arg1 == null) { // argc == 1
IRubyObject lim = context.nil;

if (sepArg != null && limArg == null) { // argc == 1
IRubyObject tmp = context.nil;

if (arg0.isNil() || !(tmp = TypeConverter.checkStringType(runtime, arg0)).isNil()) {
if (sepArg.isNil() || !(tmp = TypeConverter.checkStringType(runtime, sepArg)).isNil()) {
rs = tmp;
} else {
lim = sepArg;
}
} else if (arg0 != null && arg1 != null) { // argc >= 2
rs = arg0;
} else if (sepArg != null && limArg != null) { // argc >= 2
rs = sepArg;
if (!rs.isNil()) {
rs = rs.convertToString();
}
lim = limArg;
}

// properly encode rs
if (!rs.isNil()) {
Encoding enc_rs;

enc_rs = ((RubyString)rs).getEncoding();
enc_rs = ((RubyString) rs).getEncoding();
if (enc_io != enc_rs &&
(((RubyString)rs).scanForCodeRange() != StringSupport.CR_7BIT ||
(((RubyString)rs).size() > 0 && !enc_io.isAsciiCompatible()))) {
(((RubyString) rs).scanForCodeRange() != StringSupport.CR_7BIT ||
(((RubyString) rs).size() > 0 && !enc_io.isAsciiCompatible()))) {
if (rs == runtime.getGlobalVariables().getDefaultSeparator()) {
rs = RubyString.newStringLight(runtime, 0, enc_io);
((RubyString)rs).catAscii(NEWLINE_BYTES, 0, 1);
((RubyString) rs).catAscii(NEWLINE_BYTES, 0, 1);
}
else {
throw runtime.newArgumentError("encoding mismatch: " + enc_io + " IO with " + enc_rs + " RS");
}
}
}
return rs;
}

// MRI: prepare_getline_args, limit logic
private static long prepareGetsLimit(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
Ruby runtime = context.runtime;
IRubyObject lim = context.nil;
if (arg0 != null && arg1 == null) { // argc == 1
IRubyObject tmp = context.nil;
limit = lim.isNil() ? -1 : lim.convertToInteger().getLongValue();

if (arg0.isNil() || !(tmp = TypeConverter.checkStringType(runtime, arg0)).isNil()) {
// only separator logic
} else {
lim = arg0;
}
} else if (arg0 != null && arg1 != null) { // argc >= 2
lim = arg1;
}
return lim.isNil() ? -1 : lim.convertToInteger().getLongValue();
return getline.getline(context, self, rs, (int) limit, chomp, block);
}

private static final byte[] NEWLINE_BYTES = { (byte) '\n' };

0 comments on commit 74d0588

Please sign in to comment.