Skip to content

Commit 8a14609

Browse files
eregoneregon
committed
[GR-19220] Rope fixes (#2341)
PullRequest: truffleruby/2634
2 parents 5357e9a + d271388 commit 8a14609

31 files changed

+144
-96
lines changed

‎src/main/java/org/truffleruby/core/exception/CoreExceptions.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ public void showExceptionIfDebug(RubyClass rubyClass, Object message, Backtrace
9292
if (backtrace != null && backtrace.getStackTrace().length > 0) {
9393
from = " at " + debugBacktraceFormatter.formatLine(backtrace.getStackTrace(), 0, null);
9494
}
95+
if (RubyStringLibrary.getUncached().isRubyString(message)) {
96+
message = RubyStringLibrary.getUncached().getJavaString(message);
97+
}
9598
final String output = "Exception `" + exceptionClass + "'" + from + " - " + message + "\n";
9699
if (context.getCoreLibrary().isLoaded()) {
97100
RubyString outputString = StringOperations

‎src/main/java/org/truffleruby/core/regexp/MatchDataNodes.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.truffleruby.core.regexp.MatchDataNodesFactory.ValuesNodeFactory;
3535
import org.truffleruby.core.rope.Rope;
3636
import org.truffleruby.core.rope.RopeNodes;
37+
import org.truffleruby.core.rope.RopeOperations;
3738
import org.truffleruby.core.string.RubyString;
3839
import org.truffleruby.core.string.StringSupport;
3940
import org.truffleruby.core.string.StringUtils;
@@ -309,7 +310,7 @@ protected Object getIndexString(RubyMatchData matchData, Object index, NotProvid
309310
@CachedLibrary(limit = "2") RubyStringLibrary libIndex) {
310311
return executeGetIndex(
311312
matchData,
312-
getBackRefFromRope(matchData, index, libIndex.getRope(index)),
313+
getBackRefFromRope(matchData, libIndex.getRope(index)),
313314
NotProvided.INSTANCE);
314315
}
315316

@@ -368,12 +369,11 @@ protected RubyRegexp getRegexp(RubyMatchData matchData) {
368369

369370
@TruffleBoundary
370371
private int getBackRefFromSymbol(RubyMatchData matchData, RubySymbol index) {
371-
final Rope value = index.getRope();
372-
return getBackRefFromRope(matchData, index, value);
372+
return getBackRefFromRope(matchData, index.getRope());
373373
}
374374

375375
@TruffleBoundary
376-
private int getBackRefFromRope(RubyMatchData matchData, Object index, Rope value) {
376+
private int getBackRefFromRope(RubyMatchData matchData, Rope value) {
377377
try {
378378
return getRegexp(matchData).regex.nameToBackrefNumber(
379379
value.getBytes(),
@@ -384,7 +384,8 @@ private int getBackRefFromRope(RubyMatchData matchData, Object index, Rope value
384384
throw new RaiseException(
385385
getContext(),
386386
coreExceptions().indexError(
387-
StringUtils.format("undefined group name reference: %s", index),
387+
StringUtils
388+
.format("undefined group name reference: %s", RopeOperations.decodeRope(value)),
388389
this));
389390
}
390391
}

‎src/main/java/org/truffleruby/core/rope/ConcatRope.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,9 @@ public ConcatState(ManagedRope left, ManagedRope right, byte[] bytes) {
3333
this.bytes = bytes;
3434
}
3535

36-
public boolean isBytes() {
36+
public boolean isFlattened() {
3737
return bytes != null;
3838
}
39-
40-
public boolean isChildren() {
41-
return bytes == null;
42-
}
4339
}
4440

4541
private ManagedRope left;
@@ -69,6 +65,8 @@ private ConcatRope(
6965
int characterLength,
7066
byte[] bytes) {
7167
super(encoding, codeRange, byteLength, characterLength, bytes);
68+
assert left != null;
69+
assert right != null;
7270
this.left = left;
7371
this.right = right;
7472
}
@@ -85,19 +83,27 @@ Rope withBinaryEncoding(ConditionProfile bytesNotNull) {
8583
return withEncoding(ASCIIEncoding.INSTANCE, CodeRange.CR_VALID, byteLength(), bytesNotNull);
8684
}
8785

88-
private ConcatRope withEncoding(Encoding encoding, CodeRange codeRange, int characterLength,
86+
private Rope withEncoding(Encoding encoding, CodeRange codeRange, int characterLength,
8987
ConditionProfile bytesNotNull) {
9088
final ConcatState state = getState(bytesNotNull);
91-
return new ConcatRope(state.left, state.right, encoding, codeRange, byteLength(), characterLength, state.bytes);
89+
if (state.isFlattened()) {
90+
return RopeOperations.create(state.bytes, encoding, codeRange);
91+
} else {
92+
return new ConcatRope(state.left, state.right, encoding, codeRange, byteLength(), characterLength, null);
93+
}
9294
}
9395

9496
@Override
9597
protected byte[] getBytesSlow() {
98+
flatten();
99+
return bytes;
100+
}
101+
102+
private void flatten() {
96103
bytes = RopeOperations.flattenBytes(this);
97104
MemoryFence.storeStore();
98105
left = null;
99106
right = null;
100-
return bytes;
101107
}
102108

103109
/** Access the state in a way that prevents race conditions.

‎src/main/java/org/truffleruby/core/rope/ManagedRope.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,4 @@ public final byte[] getBytes() {
5454
return bytes;
5555
}
5656

57-
@Override
58-
public final String toString() {
59-
if (DEBUG_ROPE_BYTES) {
60-
final byte[] bytesBefore = bytes;
61-
final String string = RopeOperations.decodeOrEscapeBinaryRope(this, RopeOperations.flattenBytes(this));
62-
assert bytes == bytesBefore : "bytes should not be modified by Rope#toString() as otherwise inspecting a Rope would have a side effect";
63-
return string;
64-
} else {
65-
return RopeOperations.decodeRope(this);
66-
}
67-
}
68-
6957
}

‎src/main/java/org/truffleruby/core/rope/NativeRope.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,6 @@ public int hashCode() {
183183
return RopeOperations.hashForRange(this, 1, 0, byteLength());
184184
}
185185

186-
@Override
187-
public String toString() {
188-
return toLeafRope().toString();
189-
}
190-
191186
@Override
192187
Rope withEncoding7bit(Encoding newEncoding, ConditionProfile bytesNotNull) {
193188
return withEncoding(newEncoding);

‎src/main/java/org/truffleruby/core/rope/Rope.java

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
*/
1010
package org.truffleruby.core.rope;
1111

12+
import java.lang.management.ManagementFactory;
1213
import java.util.Arrays;
14+
import java.util.List;
1315

1416
import com.oracle.truffle.api.profiles.ConditionProfile;
1517
import org.jcodings.Encoding;
@@ -21,9 +23,6 @@ public abstract class Rope implements Comparable<Rope> {
2123
// NativeRope, RepeatingRope, 3 LeafRope, ConcatRope, SubstringRope, 1 LazyRope
2224
public static final int NUMBER_OF_CONCRETE_CLASSES = 8;
2325

24-
// Useful for debugging. Setting to true avoids ManagedRope#toString to populate bytes as a side-effect of the debugger calling toString().
25-
protected static final boolean DEBUG_ROPE_BYTES = false;
26-
2726
public final Encoding encoding;
2827
private final int byteLength;
2928
private int hashCode = 0;
@@ -161,8 +160,28 @@ public byte get(int index) {
161160
return getByteSlow(index);
162161
}
163162

164-
/** Should only be used by the parser */
165-
public final String getString() {
163+
private static boolean isJavaDebuggerAttached() {
164+
final List<String> inputArguments = ManagementFactory.getRuntimeMXBean().getInputArguments();
165+
for (String arg : inputArguments) {
166+
if (arg.contains("jdwp")) {
167+
return true;
168+
}
169+
}
170+
return false;
171+
}
172+
173+
static final boolean JAVA_DEBUGGER = isJavaDebuggerAttached();
174+
175+
/** This is designed to not have any side effects - compare to {@link #getJavaString} - but this makes it
176+
* inefficient - for debugging only */
177+
@Override
178+
public String toString() {
179+
assert JAVA_DEBUGGER : "Rope#toString() should only be called by Java debuggers, use RubyStringLibrary or RopeOperations.decodeRope() instead";
180+
return RopeOperations.decode(encoding, RopeOperations.flattenBytes(this));
181+
}
182+
183+
/** Should only be used by the parser - it has side effects */
184+
public final String getJavaString() {
166185
return RopeOperations.decodeRope(this);
167186
}
168187

‎src/main/java/org/truffleruby/core/rope/RopeCache.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ private void register(LeafRope rope) {
5454
final BytesKey key = new BytesKey(rope.getBytes(), rope.getEncoding());
5555
final Rope existing = bytesToRope.put(key, rope);
5656
if (existing != null && existing != rope) {
57-
throw new AssertionError("Duplicate Rope in RopeCache: " + existing.getString());
57+
throw new AssertionError("Duplicate Rope in RopeCache: " + existing);
5858
}
5959
}
6060

‎src/main/java/org/truffleruby/core/rope/RopeConstants.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private static Rope ascii(String string) {
112112
final LeafRope rope = new AsciiOnlyLeafRope(bytes, USASCIIEncoding.INSTANCE).computeHashCode();
113113
final Rope existing = ROPE_CONSTANTS.putIfAbsent(string, rope);
114114
if (existing != null) {
115-
throw new AssertionError("Duplicate Rope in RopeConstants: " + existing.getString());
115+
throw new AssertionError("Duplicate Rope in RopeConstants: " + existing);
116116
}
117117
return rope;
118118
}

‎src/main/java/org/truffleruby/core/rope/RopeNodes.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ protected Object debugPrintLeafRope(LeafRope rope, int currentLevel, boolean pri
747747

748748
System.err.println(StringUtils.format(
749749
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; E: %s)",
750-
printString ? rope.toString() : "<skipped>",
750+
printString ? RopeOperations.escape(rope) : "<skipped>",
751751
rope.getClass().getSimpleName(),
752752
bytesAreNull,
753753
rope.byteLength(),
@@ -768,7 +768,7 @@ protected Object debugPrintSubstringRope(SubstringRope rope, int currentLevel, b
768768

769769
System.err.println(StringUtils.format(
770770
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; O: %d; E: %s)",
771-
printString ? rope.toString() : "<skipped>",
771+
printString ? RopeOperations.escape(rope) : "<skipped>",
772772
rope.getClass().getSimpleName(),
773773
bytesAreNull,
774774
rope.byteLength(),
@@ -783,6 +783,7 @@ protected Object debugPrintSubstringRope(SubstringRope rope, int currentLevel, b
783783
}
784784

785785
@TruffleBoundary
786+
@Specialization
786787
protected Object debugPrintConcatRopeBytes(ConcatRope rope, int currentLevel, boolean printString) {
787788
printPreamble(currentLevel);
788789

@@ -791,10 +792,10 @@ protected Object debugPrintConcatRopeBytes(ConcatRope rope, int currentLevel, bo
791792
// Before the print, as `toString()` may cause the bytes to become populated.
792793
final boolean bytesAreNull = rope.getRawBytes() == null;
793794

794-
if (state.isBytes()) {
795+
if (state.isFlattened()) {
795796
System.err.println(StringUtils.format(
796797
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; E: %s)",
797-
printString ? rope.toString() : "<skipped>",
798+
printString ? RopeOperations.escape(rope) : "<skipped>",
798799
rope.getClass().getSimpleName(),
799800
bytesAreNull,
800801
rope.byteLength(),
@@ -804,7 +805,7 @@ protected Object debugPrintConcatRopeBytes(ConcatRope rope, int currentLevel, bo
804805
} else {
805806
System.err.println(StringUtils.format(
806807
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; E: %s)",
807-
printString ? rope.toString() : "<skipped>",
808+
printString ? RopeOperations.escape(rope) : "<skipped>",
808809
rope.getClass().getSimpleName(),
809810
bytesAreNull,
810811
rope.byteLength(),
@@ -829,7 +830,7 @@ protected Object debugPrintRepeatingRope(RepeatingRope rope, int currentLevel, b
829830

830831
System.err.println(StringUtils.format(
831832
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; T: %d; D: %d; E: %s)",
832-
printString ? rope.toString() : "<skipped>",
833+
printString ? RopeOperations.escape(rope) : "<skipped>",
833834
rope.getClass().getSimpleName(),
834835
bytesAreNull,
835836
rope.byteLength(),
@@ -853,7 +854,7 @@ protected Object debugPrintLazyInt(LazyIntRope rope, int currentLevel, boolean p
853854

854855
System.err.println(StringUtils.format(
855856
"%s (%s; BN: %b; BL: %d; CL: %d; CR: %s; V: %d, D: %d; E: %s)",
856-
printString ? rope.toString() : "<skipped>",
857+
printString ? RopeOperations.escape(rope) : "<skipped>",
857858
rope.getClass().getSimpleName(),
858859
bytesAreNull,
859860
rope.byteLength(),
@@ -994,7 +995,7 @@ protected int getByteRepeatingRope(RepeatingRope rope, int index,
994995
// Therefore it's important to test isChildren first, as it's possible to transition from children to bytes
995996
// but not the other way around.
996997

997-
@Specialization(guards = "state.isChildren()")
998+
@Specialization(guards = "!state.isFlattened()")
998999
protected int getByteConcatRope(ConcatRope rope, int index,
9991000
@Cached ConditionProfile stateBytesNotNull,
10001001
@Bind("rope.getState(stateBytesNotNull)") ConcatState state,
@@ -1020,7 +1021,7 @@ protected int getByteConcatRope(ConcatRope rope, int index,
10201021

10211022
// Necessary because getRawBytes() might return null, but then be populated and the children nulled
10221023
// before we get to run the other getByteConcatRope.
1023-
@Specialization(guards = "state.isBytes()")
1024+
@Specialization(guards = "state.isFlattened()")
10241025
protected int getByteConcatRope(ConcatRope rope, int index,
10251026
@Cached ConditionProfile stateBytesNotNull,
10261027
@Bind("rope.getState(stateBytesNotNull)") ConcatState state) {

‎src/main/java/org/truffleruby/core/rope/RopeOperations.java

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ public static byte[] flattenBytes(Rope rope) {
379379
final ConcatRope concatRope = (ConcatRope) current;
380380

381381
final ConcatState state = concatRope.getState();
382-
if (state.isBytes()) {
382+
if (state.isFlattened()) {
383383
// The rope got concurrently flattened between entering the iteration and reaching here,
384384
// restart the iteration from the top.
385385
workStack.push(concatRope);
@@ -508,7 +508,7 @@ static byte getByteSlow(Rope rope, int index) {
508508
} else if (rope instanceof ConcatRope) {
509509
final ConcatRope concatRope = (ConcatRope) rope;
510510
final ConcatState state = concatRope.getState();
511-
if (state.isBytes()) {
511+
if (state.isFlattened()) {
512512
// Rope got concurrently flattened.
513513
return state.bytes[index];
514514
}
@@ -580,7 +580,7 @@ class Params {
580580
} else if (rope instanceof ConcatRope) {
581581
final ConcatRope concatRope = (ConcatRope) rope;
582582
final ConcatState state = concatRope.getState();
583-
if (state.isBytes()) {
583+
if (state.isFlattened()) {
584584
// Rope got concurrently flattened.
585585
resultHash = Hashing.stringHash(state.bytes, startingHashCode, offset, length);
586586
} else {
@@ -687,4 +687,32 @@ public static boolean anyChildContains(Rope rope, String value) {
687687
// to worry about the risk of retaining a substring rope whose child contains the value.
688688
return rope.byteLength() >= value.length() && RopeOperations.decodeRope(rope).contains(value);
689689
}
690+
691+
public static String escape(Rope rope) {
692+
final StringBuilder builder = new StringBuilder();
693+
builder.append('"');
694+
695+
for (int i = 0; i < rope.byteLength(); i++) {
696+
final byte character = rope.get(i);
697+
switch (character) {
698+
case '\\':
699+
builder.append("\\");
700+
break;
701+
case '"':
702+
builder.append("\\\"");
703+
break;
704+
default:
705+
if (character >= 32 && character <= 126) {
706+
builder.append((char) character);
707+
} else {
708+
builder.append(StringUtils.format("\\x%02x", character));
709+
}
710+
break;
711+
}
712+
}
713+
714+
builder.append('"');
715+
return builder.toString();
716+
}
717+
690718
}

0 commit comments

Comments
 (0)