From 16073c546419f47c1c9a1cf8c15f7bade18813fa Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 19 Sep 2024 15:42:00 +0200 Subject: [PATCH 01/14] Implemented first version of the asReader interface on IString --- .../java/io/usethesource/vallang/IString.java | 8 ++ .../vallang/impl/primitive/StringValue.java | 124 ++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/src/main/java/io/usethesource/vallang/IString.java b/src/main/java/io/usethesource/vallang/IString.java index cee3ef89..10dc4744 100644 --- a/src/main/java/io/usethesource/vallang/IString.java +++ b/src/main/java/io/usethesource/vallang/IString.java @@ -13,6 +13,7 @@ package io.usethesource.vallang; import java.io.IOException; +import java.io.Reader; import java.io.Writer; import java.util.PrimitiveIterator.OfInt; @@ -99,6 +100,13 @@ default int getMatchFingerprint() { */ public void write(Writer w) throws IOException; + /** + * Generates a reader that can be used to stream the contents of the string + * Note, this will generate java characters, users are responsible for dealing with surrogate-pairs. + * See {@link #iterator()} for a more unicode compatible approach to iterate over the characters of an IString. + */ + public Reader asReader(); + /** * Build an iterator which generates the Unicode UTF-32 codepoints of the IString one-by-one. * @see Character for more information on Unicode UTF-32 codepoints. diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 942cad30..4f3f4ec4 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -16,6 +16,8 @@ package io.usethesource.vallang.impl.primitive; import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; import java.io.StringWriter; import java.io.Writer; import java.lang.management.ManagementFactory; @@ -279,6 +281,11 @@ public boolean isNewlineTerminated() { public IString concat(IString other) { return other; } + + @Override + public Reader asReader() { + return Reader.nullReader(); + } } private static class FullUnicodeString extends AbstractString { @@ -501,6 +508,11 @@ public void write(Writer w) throws IOException { w.write(value); } + @Override + public Reader asReader() { + return new StringReader(value); + } + @Override public void indentedWrite(Writer w, Deque whitespace, boolean indentFirstLine) throws IOException { if (value.isEmpty()) { @@ -633,6 +645,11 @@ public int nextInt() { } }; } + + @Override + public Reader asReader() { + return new StringReader(value); + } } /** @@ -1146,6 +1163,47 @@ public void write(Writer w) throws IOException { right.write(w); } + @Override + public Reader asReader() { + return new Reader() { + private Reader currentReader = left.asReader(); + private boolean readingRight = false; + + private void continueRight() throws IOException { + assert !readingRight; + currentReader.close(); + currentReader = right.asReader(); + readingRight = true; + } + + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + int result = currentReader.read(cbuf, off, len); + if (result == -1 && !readingRight) { + continueRight(); + return read(cbuf, off, len); + } + return result; + } + + @Override + public int read() throws IOException { + int result = currentReader.read(); + if (result == -1 && !readingRight) { + continueRight(); + return read(); + } + return result; + } + + @Override + public void close() throws IOException { + currentReader.close(); + } + }; + } + @Override public void indentedWrite(Writer w, Deque whitespace, boolean indentFirstLine) throws IOException { left.indentedWrite(w, whitespace, indentFirstLine); @@ -1380,6 +1438,72 @@ public void write(Writer w) throws IOException { assert indents.isEmpty(); } + @Override + public Reader asReader() { + return new Reader() { + private final OfInt chars = iterator(); + private char endedWithHalfSurrogate = 0; + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + if (off < 0 || len < 0 || len > (cbuf.length - off)) { + throw new IndexOutOfBoundsException(); + } + if (len == 0) { + return 0; + } + + int pos = off; + if (endedWithHalfSurrogate != 0) { + cbuf[pos++] = endedWithHalfSurrogate; + endedWithHalfSurrogate = 0; + } + int endPos = off + len; + while (pos <= endPos) { + if (!chars.hasNext()) { + break; + } + int nextChar = chars.nextInt(); + if (Character.isBmpCodePoint(nextChar)) { + cbuf[pos++] = (char)nextChar; + } else { + cbuf[pos++] = Character.highSurrogate(nextChar); + char lowSide = Character.lowSurrogate(nextChar); + if (pos <= endPos) { + cbuf[pos++] = lowSide; + } else { + endedWithHalfSurrogate = lowSide; + } + } + } + int written = pos - off; + return written == 0 ? /* EOF */ -1 : written; + } + + @Override + public int read() throws IOException { + if (endedWithHalfSurrogate != 0) { + int result = endedWithHalfSurrogate; + endedWithHalfSurrogate = 0; + return result; + } + if (chars.hasNext()) { + int nextChar = chars.nextInt(); + if (Character.isBmpCodePoint(nextChar)) { + return nextChar; + } else { + endedWithHalfSurrogate = Character.lowSurrogate(nextChar); + return Character.highSurrogate(nextChar); + } + } + return -1; + } + @Override + public void close() throws IOException { + } + }; + } + @Override public void indentedWrite(Writer w, Deque whitespace, boolean indentFirstLine) throws IOException { if (flattened != null) { From 21c2bf47fb285d623d00d0dce450767daed7df8c Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 19 Sep 2024 16:18:27 +0200 Subject: [PATCH 02/14] Added tests for reader and writers --- .../vallang/basic/BasicValueSmokeTest.java | 54 +++++++++++++++++++ .../basic/LazyStringOperationsTest.java | 29 +++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java index 572cea69..57763119 100644 --- a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java @@ -174,6 +174,27 @@ public void testStringWrite(IValueFactory vf) { } } + @ParameterizedTest @ArgumentsSource(ValueProvider.class) + public void testStringRead(IValueFactory vf) { + Random rnd = new Random(); + + for (int i = 0; i < 1000; i++) { + IString testString = vf.string(RandomUtil.string(rnd, rnd.nextInt(200))); + var result = new StringBuilder(); + try (var r = testString.asReader()) { + char[] buffer = new char[1024]; + int read = 0; + while ((read = r.read(buffer)) > 0) { + result.append(buffer, 0, read); + } + } catch (IOException e) { + fail(e.getMessage()); + } + + assertEqual(testString, vf.string(result.toString())); + } + } + @ParameterizedTest @ArgumentsSource(ValueProvider.class) public void testStringEmptyWrite(IValueFactory vf) { IString testString = vf.string(""); @@ -284,6 +305,7 @@ private void checkIndent(IValueFactory vf, String indent, String newline, boolea assertSimilarIteration(indentedDirect, indentedConcatTree); assertEqualLength(indentedDirect, indentedConcatTree); assertEqual(indentedDirect, indentedConcatTree); + assertEqualWriteAndRead(indentedDirect, indentedConcatTree); assertEquals(indentedDirect.hashCode(), indentedConcatTree.hashCode()); // these modify internal structure as a side-effect, so after this we test the above again! @@ -298,6 +320,7 @@ private void checkIndent(IValueFactory vf, String indent, String newline, boolea assertSimilarIteration(vf.string(expected), indentedConcatTree); assertSimilarIteration(indentedDirect, indentedConcatTree); assertEqual(indentedDirect, indentedConcatTree); + assertEqualWriteAndRead(indentedDirect, indentedConcatTree); assertEquals(indentedDirect.hashCode(), indentedConcatTree.hashCode()); // basic tests showing lazy versus eager indentation should have the same semantics: @@ -306,6 +329,7 @@ private void checkIndent(IValueFactory vf, String indent, String newline, boolea assertSimilarIteration(vf.string(expectedTwice), indentedDirectTwice); assertSimilarIteration(vf.string(expectedTwice), indentedConcatTreeTwice); assertEqual(indentedDirectTwice, indentedConcatTreeTwice); + assertEqualWriteAndRead(indentedDirectTwice, indentedConcatTreeTwice); assertSimilarIteration(indentedDirectTwice, indentedConcatTreeTwice); assertEquals(indentedDirectTwice.hashCode(), indentedConcatTreeTwice.hashCode()); @@ -326,10 +350,40 @@ private void checkIndent(IValueFactory vf, String indent, String newline, boolea assertSimilarIteration(vf.string(expectedTwice), indentedDirectTwice); assertSimilarIteration(vf.string(expectedTwice), indentedConcatTreeTwice); assertEqual(indentedDirectTwice, indentedConcatTreeTwice); + assertEqualWriteAndRead(indentedDirectTwice, indentedConcatTreeTwice); assertSimilarIteration(indentedDirectTwice, indentedConcatTreeTwice); assertEquals(indentedDirectTwice.hashCode(), indentedConcatTreeTwice.hashCode()); } + + private String writerToString(IString a) { + try { + var result = new StringWriter(); + a.write(result); + return result.toString(); + } catch (IOException e) { + fail("IString::write failed", e); + return ""; + } + } + + private String readerToString(IString a) { + try (var r = a.asReader()) { + var result = new StringWriter(); + a.asReader().transferTo(result); + return result.toString(); + } catch (IOException e) { + fail("IString::asReader failed", e); + return ""; + } + } + + + private void assertEqualWriteAndRead(IString one, IString two) { + assertEquals(writerToString(one), writerToString(two), "IString::write had different results"); + assertEquals(readerToString(one), readerToString(two), "IString::asReader had different results"); + } + private void assertEqualCharAt(IString one, IString two) { assertEquals(one, two); diff --git a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java index 02fb70ee..17dc85ef 100644 --- a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java +++ b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.fail; import java.io.IOException; +import java.io.Reader; import java.io.StringWriter; import java.util.Random; @@ -192,8 +193,22 @@ public void testStringReplace(IValueFactory vf) { vf.string("abcdefxygh").concat(str.substring(8))); } + private String fromReader(Reader r) throws IOException { + try { + var result = new StringBuilder(); + char[] buffer = new char[8 * 1024]; + int read = 0; + while ((read = r.read(buffer)) > 0) { + result.append(buffer, 0, read); + } + return result.toString(); + } finally { + r.close(); + } + } + @ParameterizedTest @ArgumentsSource(ValueProvider.class) - public void neverRunOutOfStack(IValueFactory vf) { + public void neverRunOutOfStack(IValueFactory vf) throws IOException { int outofStack = 100000; // first we have to know for sure that we would run out of stack with @see @@ -216,6 +231,16 @@ public void neverRunOutOfStack(IValueFactory vf) { // TODO Auto-generated catch block fail("unexpected IO:" + e); } + try { + fromReader(v.asReader()); + fail("this should run out of stack"); + } catch (StackOverflowError e) { + // yes, that is what is expected + } catch (IOException e) { + // TODO Auto-generated catch block + fail("unexpected IO:" + e); + } + } finally { StringValue.resetMaxFlatString(); StringValue.resetMaxUnbalance(); @@ -231,6 +256,8 @@ public void neverRunOutOfStack(IValueFactory vf) { try { new StringWriter().write(v.toString()); // do not remove this, this is the test assertTrue(true); + fromReader(v.asReader()); + assertTrue(true); } catch (StackOverflowError e) { fail("the tree balancer should have avoided a stack overflow"); } From e289490e18bf66a4e950781db0c8aaeab351afa4 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 19 Sep 2024 16:26:40 +0200 Subject: [PATCH 03/14] More tests for the reader and write equality --- .../java/io/usethesource/vallang/basic/BasicValueSmokeTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java index 57763119..76413036 100644 --- a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java @@ -382,6 +382,8 @@ private String readerToString(IString a) { private void assertEqualWriteAndRead(IString one, IString two) { assertEquals(writerToString(one), writerToString(two), "IString::write had different results"); assertEquals(readerToString(one), readerToString(two), "IString::asReader had different results"); + assertEquals(one.getValue(), writerToString(one), "IString::write should be the same as getValue"); + assertEquals(one.getValue(), readerToString(one), "IString::asReader should be the same as getValue"); } private void assertEqualCharAt(IString one, IString two) { From a7cf69aec2199b9f6401b2f66183b33820d516b0 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 19 Sep 2024 16:46:07 +0200 Subject: [PATCH 04/14] Improved test coverage of the asReader function and fixed a bug the test uncovered --- .../vallang/impl/primitive/StringValue.java | 4 +-- .../vallang/basic/BasicValueSmokeTest.java | 30 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 4f3f4ec4..d8c65ff9 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1459,7 +1459,7 @@ public int read(char[] cbuf, int off, int len) throws IOException { endedWithHalfSurrogate = 0; } int endPos = off + len; - while (pos <= endPos) { + while (pos < endPos) { if (!chars.hasNext()) { break; } @@ -1469,7 +1469,7 @@ public int read(char[] cbuf, int off, int len) throws IOException { } else { cbuf[pos++] = Character.highSurrogate(nextChar); char lowSide = Character.lowSurrogate(nextChar); - if (pos <= endPos) { + if (pos < endPos) { cbuf[pos++] = lowSide; } else { endedWithHalfSurrogate = lowSide; diff --git a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java index 76413036..bf5ba410 100644 --- a/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java +++ b/src/test/java/io/usethesource/vallang/basic/BasicValueSmokeTest.java @@ -378,12 +378,38 @@ private String readerToString(IString a) { } } + private String readerSlowly(IString a) { + try (var r = a.asReader()) { + var result = new StringBuilder(); + while (true) { + int oneChar = r.read(); + if (oneChar == -1) { + break; + } + result.appendCodePoint(oneChar); + var buf = new char[3]; + int read = r.read(buf); + if (read == -1) { + break; + } + result.append(buf, 0, read); + } + return result.toString(); + } catch (IOException e) { + fail("IString::asReader failed", e); + return ""; + } + + } + private void assertEqualWriteAndRead(IString one, IString two) { - assertEquals(writerToString(one), writerToString(two), "IString::write had different results"); - assertEquals(readerToString(one), readerToString(two), "IString::asReader had different results"); assertEquals(one.getValue(), writerToString(one), "IString::write should be the same as getValue"); assertEquals(one.getValue(), readerToString(one), "IString::asReader should be the same as getValue"); + assertEquals(writerToString(one), writerToString(two), "IString::write had different results"); + assertEquals(readerToString(one), readerToString(two), "IString::asReader had different results"); + assertEquals(one.getValue(), readerSlowly(one), "IString::asReader had different results depending on buffer size"); + assertEquals(two.getValue(), readerSlowly(two), "IString::asReader had different results depending on buffer size"); } private void assertEqualCharAt(IString one, IString two) { From 206676ffed7c3816118e0bcbee2d0a469daa3e85 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Thu, 19 Sep 2024 16:59:45 +0200 Subject: [PATCH 05/14] Simplified hanlding of surrogate pairs in the loop --- .../vallang/impl/primitive/StringValue.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index d8c65ff9..73c28329 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1442,7 +1442,7 @@ public void write(Writer w) throws IOException { public Reader asReader() { return new Reader() { private final OfInt chars = iterator(); - private char endedWithHalfSurrogate = 0; + private char queuedLowSurrogate = 0; @Override public int read(char[] cbuf, int off, int len) throws IOException { @@ -1454,12 +1454,15 @@ public int read(char[] cbuf, int off, int len) throws IOException { } int pos = off; - if (endedWithHalfSurrogate != 0) { - cbuf[pos++] = endedWithHalfSurrogate; - endedWithHalfSurrogate = 0; - } int endPos = off + len; + char lowSurrogate = queuedLowSurrogate; + queuedLowSurrogate = 0; while (pos < endPos) { + if (lowSurrogate != 0) { + cbuf[pos++] = lowSurrogate; + lowSurrogate = 0; + continue; + } if (!chars.hasNext()) { break; } @@ -1468,23 +1471,19 @@ public int read(char[] cbuf, int off, int len) throws IOException { cbuf[pos++] = (char)nextChar; } else { cbuf[pos++] = Character.highSurrogate(nextChar); - char lowSide = Character.lowSurrogate(nextChar); - if (pos < endPos) { - cbuf[pos++] = lowSide; - } else { - endedWithHalfSurrogate = lowSide; - } + lowSurrogate = Character.lowSurrogate(nextChar); } } + queuedLowSurrogate = lowSurrogate; int written = pos - off; return written == 0 ? /* EOF */ -1 : written; } @Override public int read() throws IOException { - if (endedWithHalfSurrogate != 0) { - int result = endedWithHalfSurrogate; - endedWithHalfSurrogate = 0; + if (queuedLowSurrogate != 0) { + int result = queuedLowSurrogate; + queuedLowSurrogate = 0; return result; } if (chars.hasNext()) { @@ -1492,7 +1491,7 @@ public int read() throws IOException { if (Character.isBmpCodePoint(nextChar)) { return nextChar; } else { - endedWithHalfSurrogate = Character.lowSurrogate(nextChar); + queuedLowSurrogate = Character.lowSurrogate(nextChar); return Character.highSurrogate(nextChar); } } From 589aa9f587bb4455d8d5291a1b6658c2fa28d5eb Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 20 Sep 2024 13:06:08 +0200 Subject: [PATCH 06/14] Rewrote the readers to use the CharBuffer iterator style --- .../vallang/impl/primitive/StringValue.java | 258 +++++++++++------- .../basic/LazyStringOperationsTest.java | 10 - 2 files changed, 152 insertions(+), 116 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 73c28329..ab0ad485 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -27,15 +27,14 @@ import java.time.Duration; import java.time.temporal.ChronoUnit; import java.util.ArrayDeque; +import java.util.Collections; import java.util.Deque; import java.util.Iterator; import java.util.NoSuchElementException; import java.util.PrimitiveIterator; import java.util.PrimitiveIterator.OfInt; - import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.Nullable; - import io.usethesource.vallang.IString; import io.usethesource.vallang.IValueFactory; import io.usethesource.vallang.impl.persistent.ValueFactory; @@ -286,6 +285,11 @@ public IString concat(IString other) { public Reader asReader() { return Reader.nullReader(); } + + @Override + public Iterator iterateParts() { + return Collections.emptyIterator(); + } } private static class FullUnicodeString extends AbstractString { @@ -591,6 +595,11 @@ public int nextInt() { } }; } + + @Override + public Iterator iterateParts() { + return Collections.singleton(CharBuffer.wrap(value)).iterator(); + } } /** @@ -650,6 +659,11 @@ public int nextInt() { public Reader asReader() { return new StringReader(value); } + + @Override + public Iterator iterateParts() { + return Collections.singleton(CharBuffer.wrap(value)).iterator(); + } } /** @@ -825,6 +839,8 @@ default AbstractString rotateRightLeft() { default AbstractString rotateLeftRight() { return (AbstractString) this; } + + Iterator iterateParts(); } private abstract static class AbstractString implements IString, IStringTreeNode, IIndentableString { @@ -980,6 +996,34 @@ protected final int hashCode(int prefixCode) { } abstract boolean hasNonBMPCodePoints(); + + public abstract Iterator iterateParts(); + + @Override + public Reader asReader() { + return new Reader() { + final Iterator parts = iterateParts(); + CharBuffer currentBuffer = CharBuffer.allocate(0); + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + var actualBuffer = currentBuffer; + while (!actualBuffer.hasRemaining()) { + if (!parts.hasNext()) { + return -1; + } + actualBuffer = currentBuffer = parts.next(); + } + int actualLength = Math.min(len, actualBuffer.remaining()); + actualBuffer.get(cbuf, off, actualLength); + return actualLength; + } + + @Override + public void close() throws IOException { + } + }; + } } private static class LazyConcatString extends AbstractString { @@ -1163,47 +1207,8 @@ public void write(Writer w) throws IOException { right.write(w); } - @Override - public Reader asReader() { - return new Reader() { - private Reader currentReader = left.asReader(); - private boolean readingRight = false; - - private void continueRight() throws IOException { - assert !readingRight; - currentReader.close(); - currentReader = right.asReader(); - readingRight = true; - } - @Override - public int read(char[] cbuf, int off, int len) throws IOException { - int result = currentReader.read(cbuf, off, len); - if (result == -1 && !readingRight) { - continueRight(); - return read(cbuf, off, len); - } - return result; - } - - @Override - public int read() throws IOException { - int result = currentReader.read(); - if (result == -1 && !readingRight) { - continueRight(); - return read(); - } - return result; - } - - @Override - public void close() throws IOException { - currentReader.close(); - } - }; - } - @Override public void indentedWrite(Writer w, Deque whitespace, boolean indentFirstLine) throws IOException { left.indentedWrite(w, whitespace, indentFirstLine); @@ -1262,6 +1267,49 @@ public int nextInt() { }; } + @Override + public Iterator iterateParts() { + return new Iterator<> () { + final Deque todo = new ArrayDeque<>(depth); + Iterator currentLeaf = leftmostLeafIteratorParts(todo, LazyConcatString.this); + @Override + public boolean hasNext() { + return currentLeaf.hasNext(); /* || !todo.isEmpty() is unnecessary due to post-condition of nextInt() */ + } + @Override + public CharBuffer next() { + var next = currentLeaf.next(); + + if (!currentLeaf.hasNext() && !todo.isEmpty()) { + // now we back track to the previous node we went left from, + // take the right branch and continue with its first leaf: + currentLeaf = leftmostLeafIteratorParts(todo, todo.pop()); + } + + assert currentLeaf.hasNext() || todo.isEmpty(); + return next; + } + }; + } + + /** + * Static helper function for the iterator() method. + * + * It finds the left-most leaf of the tree, and collects + * the path of nodes to this leaf as a side-effect in the todo + * stack. + */ + private static Iterator leftmostLeafIteratorParts(Deque todo, IStringTreeNode start) { + IStringTreeNode cur = start; + + while (cur.depth() > 1) { + todo.push(cur.right()); + cur = cur.left(); + } + + return cur.iterateParts(); + } + /** * Static helper function for the iterator() method. * @@ -1390,6 +1438,69 @@ public int nextInt() { }; } + @Override + public Iterator iterateParts() { + if (flattened != null) { + return flattened.iterateParts(); + } + var indentBuffer = CharBuffer.wrap(indent.getValue()); + return new Iterator<>() { + final Iterator content = wrapped.iterateParts(); + CharBuffer active = CharBuffer.allocate(0); + boolean indentNext = indentFirstLine; + + @Override + public boolean hasNext() { + return indentNext || content.hasNext() || active.hasRemaining(); + } + + private CharBuffer nextTillNewlineOrEndOfBuffer() { + int start = active.position(); + int end = start + active.remaining(); + int cur = start; + while (cur < end) { + if (active.get(cur) == NEWLINE) { + cur++; + indentNext = true; + break; + } + cur++; + } + if (cur != end) { + var result = active.duplicate(); + result.limit(cur); + active.position(cur); + return result; + } + else { + // end of the buffer + var result = active; + if (content.hasNext()) { + active = content.next(); + } + else { + // end of the stream + indentNext = false; + active = CharBuffer.allocate(0); + } + return result; + } + } + + @Override + public CharBuffer next() { + if (indentNext) { + indentNext = false; + return indentBuffer.asReadOnlyBuffer(); + } + // okay so no indent to send + // now we should give the next char-buffer till the next newline + return nextTillNewlineOrEndOfBuffer(); + } + + }; + } + @Override public IString reverse() { return applyIndentation().reverse(); @@ -1438,71 +1549,6 @@ public void write(Writer w) throws IOException { assert indents.isEmpty(); } - @Override - public Reader asReader() { - return new Reader() { - private final OfInt chars = iterator(); - private char queuedLowSurrogate = 0; - - @Override - public int read(char[] cbuf, int off, int len) throws IOException { - if (off < 0 || len < 0 || len > (cbuf.length - off)) { - throw new IndexOutOfBoundsException(); - } - if (len == 0) { - return 0; - } - - int pos = off; - int endPos = off + len; - char lowSurrogate = queuedLowSurrogate; - queuedLowSurrogate = 0; - while (pos < endPos) { - if (lowSurrogate != 0) { - cbuf[pos++] = lowSurrogate; - lowSurrogate = 0; - continue; - } - if (!chars.hasNext()) { - break; - } - int nextChar = chars.nextInt(); - if (Character.isBmpCodePoint(nextChar)) { - cbuf[pos++] = (char)nextChar; - } else { - cbuf[pos++] = Character.highSurrogate(nextChar); - lowSurrogate = Character.lowSurrogate(nextChar); - } - } - queuedLowSurrogate = lowSurrogate; - int written = pos - off; - return written == 0 ? /* EOF */ -1 : written; - } - - @Override - public int read() throws IOException { - if (queuedLowSurrogate != 0) { - int result = queuedLowSurrogate; - queuedLowSurrogate = 0; - return result; - } - if (chars.hasNext()) { - int nextChar = chars.nextInt(); - if (Character.isBmpCodePoint(nextChar)) { - return nextChar; - } else { - queuedLowSurrogate = Character.lowSurrogate(nextChar); - return Character.highSurrogate(nextChar); - } - } - return -1; - } - @Override - public void close() throws IOException { - } - }; - } - @Override public void indentedWrite(Writer w, Deque whitespace, boolean indentFirstLine) throws IOException { if (flattened != null) { diff --git a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java index 17dc85ef..2dd30c39 100644 --- a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java +++ b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java @@ -231,16 +231,6 @@ public void neverRunOutOfStack(IValueFactory vf) throws IOException { // TODO Auto-generated catch block fail("unexpected IO:" + e); } - try { - fromReader(v.asReader()); - fail("this should run out of stack"); - } catch (StackOverflowError e) { - // yes, that is what is expected - } catch (IOException e) { - // TODO Auto-generated catch block - fail("unexpected IO:" + e); - } - } finally { StringValue.resetMaxFlatString(); StringValue.resetMaxUnbalance(); From ea2101ac16e2db38a16cf961e0f0763fe1f63bb4 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 20 Sep 2024 14:01:43 +0200 Subject: [PATCH 07/14] Make sure to fill the buffers as much as we can --- .../vallang/impl/primitive/StringValue.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index ab0ad485..48b4e08c 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1005,18 +1005,34 @@ public Reader asReader() { final Iterator parts = iterateParts(); CharBuffer currentBuffer = CharBuffer.allocate(0); - @Override - public int read(char[] cbuf, int off, int len) throws IOException { + private CharBuffer getBuffer() { var actualBuffer = currentBuffer; while (!actualBuffer.hasRemaining()) { if (!parts.hasNext()) { - return -1; + return actualBuffer; } actualBuffer = currentBuffer = parts.next(); } - int actualLength = Math.min(len, actualBuffer.remaining()); - actualBuffer.get(cbuf, off, actualLength); - return actualLength; + return actualBuffer; + } + + @Override + public int read(char[] cbuf, int off, int len) throws IOException { + if (off < 0 || len < 0 || len > cbuf.length + off) { + throw new IndexOutOfBoundsException(); + } + int written = 0; + while (written < len) { + // we try to read as much as we can from the chunks of the buffer + var actualBuffer = getBuffer(); + if (!actualBuffer.hasRemaining()) { + break; + } + int toRead = Math.min(len - written, actualBuffer.remaining()); + actualBuffer.get(cbuf, off + written, toRead); + written += toRead; + } + return written == 0 ? -1 : written; } @Override From 1bc04adaffa88e381d5df7b8ccb88670aaa02022 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Fri, 20 Sep 2024 14:08:33 +0200 Subject: [PATCH 08/14] Simplified buffer code --- .../vallang/impl/primitive/StringValue.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 48b4e08c..5dce7d71 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1021,18 +1021,15 @@ public int read(char[] cbuf, int off, int len) throws IOException { if (off < 0 || len < 0 || len > cbuf.length + off) { throw new IndexOutOfBoundsException(); } - int written = 0; - while (written < len) { - // we try to read as much as we can from the chunks of the buffer + var target = CharBuffer.wrap(cbuf, off, len); + while (target.hasRemaining()) { var actualBuffer = getBuffer(); if (!actualBuffer.hasRemaining()) { break; } - int toRead = Math.min(len - written, actualBuffer.remaining()); - actualBuffer.get(cbuf, off + written, toRead); - written += toRead; + actualBuffer.read(target); } - return written == 0 ? -1 : written; + return target.position() == off ? -1 : (len - target.remaining()); } @Override From f1070127ec15870bd1a0f05eb62fd5dc5b8b01ee Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sat, 21 Sep 2024 09:19:29 +0200 Subject: [PATCH 09/14] Refactored away clone --- .../vallang/impl/primitive/StringValue.java | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 5dce7d71..e52c85a8 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1257,7 +1257,7 @@ public AbstractString rotateLeftRight() { public OfInt iterator() { return new OfInt() { final Deque todo = new ArrayDeque<>(depth); - OfInt currentLeaf = leftmostLeafIterator(todo, LazyConcatString.this); + OfInt currentLeaf = leftmostLeaf(todo, LazyConcatString.this).iterator(); @Override public boolean hasNext() { @@ -1271,7 +1271,7 @@ public int nextInt() { if (!currentLeaf.hasNext() && !todo.isEmpty()) { // now we back track to the previous node we went left from, // take the right branch and continue with its first leaf: - currentLeaf = leftmostLeafIterator(todo, todo.pop()); + currentLeaf = leftmostLeaf(todo, todo.pop()).iterator(); } assert currentLeaf.hasNext() || todo.isEmpty(); @@ -1284,7 +1284,7 @@ public int nextInt() { public Iterator iterateParts() { return new Iterator<> () { final Deque todo = new ArrayDeque<>(depth); - Iterator currentLeaf = leftmostLeafIteratorParts(todo, LazyConcatString.this); + Iterator currentLeaf = leftmostLeaf(todo, LazyConcatString.this).iterateParts(); @Override public boolean hasNext() { return currentLeaf.hasNext(); /* || !todo.isEmpty() is unnecessary due to post-condition of nextInt() */ @@ -1296,7 +1296,7 @@ public CharBuffer next() { if (!currentLeaf.hasNext() && !todo.isEmpty()) { // now we back track to the previous node we went left from, // take the right branch and continue with its first leaf: - currentLeaf = leftmostLeafIteratorParts(todo, todo.pop()); + currentLeaf = leftmostLeaf(todo, todo.pop()).iterateParts(); } assert currentLeaf.hasNext() || todo.isEmpty(); @@ -1312,7 +1312,7 @@ public CharBuffer next() { * the path of nodes to this leaf as a side-effect in the todo * stack. */ - private static Iterator leftmostLeafIteratorParts(Deque todo, IStringTreeNode start) { + private static IStringTreeNode leftmostLeaf(Deque todo, IStringTreeNode start) { IStringTreeNode cur = start; while (cur.depth() > 1) { @@ -1320,26 +1320,9 @@ private static Iterator leftmostLeafIteratorParts(Deque todo, IStringTreeNode start) { - IStringTreeNode cur = start; - - while (cur.depth() > 1) { - todo.push(cur.right()); - cur = cur.left(); - } - - return cur.iterator(); - } } private static class IndentedString extends AbstractString { From 990980952cf0556ed3a0c32756a12180d022dc21 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sat, 21 Sep 2024 09:52:10 +0200 Subject: [PATCH 10/14] Further refactoring of common iterator pattern --- .../vallang/impl/primitive/StringValue.java | 82 +++++++++++++------ 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index e52c85a8..4c064c9a 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -33,6 +33,7 @@ import java.util.NoSuchElementException; import java.util.PrimitiveIterator; import java.util.PrimitiveIterator.OfInt; +import java.util.function.Function; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.Nullable; import io.usethesource.vallang.IString; @@ -1256,26 +1257,16 @@ public AbstractString rotateLeftRight() { @Override public OfInt iterator() { return new OfInt() { - final Deque todo = new ArrayDeque<>(depth); - OfInt currentLeaf = leftmostLeaf(todo, LazyConcatString.this).iterator(); + IteratorOfIterators current = new IteratorOfIterators<>(leftMostIterator(), IStringTreeNode::iterator); @Override public boolean hasNext() { - return currentLeaf.hasNext(); /* || !todo.isEmpty() is unnecessary due to post-condition of nextInt() */ + return current.hasNext(); } @Override public int nextInt() { - int next = currentLeaf.nextInt(); - - if (!currentLeaf.hasNext() && !todo.isEmpty()) { - // now we back track to the previous node we went left from, - // take the right branch and continue with its first leaf: - currentLeaf = leftmostLeaf(todo, todo.pop()).iterator(); - } - - assert currentLeaf.hasNext() || todo.isEmpty(); - return next; + return current.currentIterator().nextInt(); } }; } @@ -1283,24 +1274,67 @@ public int nextInt() { @Override public Iterator iterateParts() { return new Iterator<> () { - final Deque todo = new ArrayDeque<>(depth); - Iterator currentLeaf = leftmostLeaf(todo, LazyConcatString.this).iterateParts(); + IteratorOfIterators> current = new IteratorOfIterators<>(leftMostIterator(), IStringTreeNode::iterateParts); + @Override public boolean hasNext() { - return currentLeaf.hasNext(); /* || !todo.isEmpty() is unnecessary due to post-condition of nextInt() */ + return current.hasNext(); } + @Override public CharBuffer next() { - var next = currentLeaf.next(); + return current.currentIterator().next(); + } + }; + } - if (!currentLeaf.hasNext() && !todo.isEmpty()) { - // now we back track to the previous node we went left from, - // take the right branch and continue with its first leaf: - currentLeaf = leftmostLeaf(todo, todo.pop()).iterateParts(); - } + private static class IteratorOfIterators> { + private final Iterator base; + private final Function nested; + private @Nullable T current = null; + + IteratorOfIterators(Iterator base, Function nested) { + this.base = base; + this.nested = nested; + } + + boolean hasNext() { + return base.hasNext() || (current != null && current.hasNext()); + } + + T currentIterator() { + if (current == null || !current.hasNext()) { + current = nested.apply(base.next()); + } + return current; + } + + } - assert currentLeaf.hasNext() || todo.isEmpty(); - return next; + private Iterator leftMostIterator() { + return new Iterator<>() { + final Deque todo = new ArrayDeque<>(depth); + IStringTreeNode nextNode = leftmostLeaf(todo, LazyConcatString.this); + + + @Override + public boolean hasNext() { + return nextNode != null; /* || !todo.isEmpty() is unnecessary due to post-condition of next() */ + } + + @Override + public IStringTreeNode next() { + var result = nextNode; + if (result == null) { + throw new NoSuchElementException(); + } + if (todo.isEmpty()) { + nextNode = null; + } + else { + nextNode = leftmostLeaf(todo, todo.pop()); + } + return result; } }; } From 82c9844b670c7d9b93005f8f85502cfa5e964053 Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sun, 22 Sep 2024 11:28:00 +0200 Subject: [PATCH 11/14] Improved some tests --- .../vallang/basic/LazyStringOperationsTest.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java index 2dd30c39..2e269adf 100644 --- a/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java +++ b/src/test/java/io/usethesource/vallang/basic/LazyStringOperationsTest.java @@ -1,5 +1,7 @@ package io.usethesource.vallang.basic; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -92,11 +94,11 @@ public void testEquals(IValueFactory vf) { IString y = vf.string("abcdefgh"); IString z = vf.string("abcdefgi"); - assertTrue(x.hashCode() == y.hashCode()); - assertTrue(x.equals(y)); - assertTrue(y.equals(x)); - assertTrue(!z.equals(x)); - assertTrue(x.substring(0, 0).equals(vf.string(""))); + assertEquals(x.hashCode(), y.hashCode()); + assertEquals(x, y); + assertEquals(y, x); + assertNotEquals(z, x); + assertEquals(x.substring(0, 0), vf.string("")); } finally { StringValue.resetMaxFlatString(); StringValue.resetMaxUnbalance(); From b47627ce5544f19553fc6ba4865d58ce868d1cfc Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sun, 22 Sep 2024 11:28:35 +0200 Subject: [PATCH 12/14] Refactored iterator of iterator into a single class that does both --- .../vallang/impl/primitive/StringValue.java | 101 +++++++----------- 1 file changed, 36 insertions(+), 65 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 4c064c9a..26599bac 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1257,16 +1257,16 @@ public AbstractString rotateLeftRight() { @Override public OfInt iterator() { return new OfInt() { - IteratorOfIterators current = new IteratorOfIterators<>(leftMostIterator(), IStringTreeNode::iterator); + final InOrderIterator it = new InOrderIterator<>(IStringTreeNode::iterator); @Override public boolean hasNext() { - return current.hasNext(); + return it.getActive().hasNext(); } @Override public int nextInt() { - return current.currentIterator().nextInt(); + return it.getActive().nextInt(); } }; } @@ -1274,87 +1274,58 @@ public int nextInt() { @Override public Iterator iterateParts() { return new Iterator<> () { - IteratorOfIterators> current = new IteratorOfIterators<>(leftMostIterator(), IStringTreeNode::iterateParts); + final InOrderIterator> it = new InOrderIterator<>(IStringTreeNode::iterateParts); @Override public boolean hasNext() { - return current.hasNext(); + return it.getActive().hasNext(); } @Override public CharBuffer next() { - return current.currentIterator().next(); + return it.getActive().next(); } }; } - private static class IteratorOfIterators> { - private final Iterator base; - private final Function nested; - private @Nullable T current = null; - - IteratorOfIterators(Iterator base, Function nested) { - this.base = base; - this.nested = nested; - } - - boolean hasNext() { - return base.hasNext() || (current != null && current.hasNext()); + /** + * An in order traversel of the leafs of the concat tree. + * We then for every leaf call the desired iterator, and replace it when the next when it's consumed + */ + private class InOrderIterator> { + private final Deque todo = new ArrayDeque<>(depth); + private final Function getActualIterator; + private T activeIterator; + + InOrderIterator( Function getActualIterator) { + this.getActualIterator = getActualIterator; + activeIterator = getActualIterator.apply(leftmostLeaf(todo, LazyConcatString.this)); } - T currentIterator() { - if (current == null || !current.hasNext()) { - current = nested.apply(base.next()); + T getActive() { + while (!activeIterator.hasNext() && !todo.isEmpty()) { + activeIterator = getActualIterator.apply(leftmostLeaf(todo, todo.pop())); } - return current; + return activeIterator; } - } - - private Iterator leftMostIterator() { - return new Iterator<>() { - final Deque todo = new ArrayDeque<>(depth); - IStringTreeNode nextNode = leftmostLeaf(todo, LazyConcatString.this); - - - @Override - public boolean hasNext() { - return nextNode != null; /* || !todo.isEmpty() is unnecessary due to post-condition of next() */ + /** + * helper function for the iterator() method. + * + * It finds the left-most leaf of the tree, and collects + * the path of nodes to this leaf as a side-effect in the todo + * stack. + */ + private IStringTreeNode leftmostLeaf(Deque todo, IStringTreeNode start) { + IStringTreeNode cur = start; + + while (cur.depth() > 1) { + todo.push(cur.right()); + cur = cur.left(); } - @Override - public IStringTreeNode next() { - var result = nextNode; - if (result == null) { - throw new NoSuchElementException(); - } - if (todo.isEmpty()) { - nextNode = null; - } - else { - nextNode = leftmostLeaf(todo, todo.pop()); - } - return result; - } - }; - } - - /** - * Static helper function for the iterator() method. - * - * It finds the left-most leaf of the tree, and collects - * the path of nodes to this leaf as a side-effect in the todo - * stack. - */ - private static IStringTreeNode leftmostLeaf(Deque todo, IStringTreeNode start) { - IStringTreeNode cur = start; - - while (cur.depth() > 1) { - todo.push(cur.right()); - cur = cur.left(); + return cur; } - - return cur; } } From f70c2009d0893eff4cea8afb79dccd0c528f43fb Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sun, 22 Sep 2024 16:50:18 +0200 Subject: [PATCH 13/14] Helping CF realize things are initialized --- .../vallang/impl/primitive/StringValue.java | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index 26599bac..ab9b5190 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1293,12 +1293,13 @@ public CharBuffer next() { * We then for every leaf call the desired iterator, and replace it when the next when it's consumed */ private class InOrderIterator> { - private final Deque todo = new ArrayDeque<>(depth); + private final Deque todo; private final Function getActualIterator; private T activeIterator; InOrderIterator( Function getActualIterator) { this.getActualIterator = getActualIterator; + todo = new ArrayDeque<>(depth); activeIterator = getActualIterator.apply(leftmostLeaf(todo, LazyConcatString.this)); } @@ -1309,24 +1310,24 @@ T getActive() { return activeIterator; } - /** - * helper function for the iterator() method. - * - * It finds the left-most leaf of the tree, and collects - * the path of nodes to this leaf as a side-effect in the todo - * stack. - */ - private IStringTreeNode leftmostLeaf(Deque todo, IStringTreeNode start) { - IStringTreeNode cur = start; - - while (cur.depth() > 1) { - todo.push(cur.right()); - cur = cur.left(); - } + } + /** + * helper function for the iterator() method. + * + * It finds the left-most leaf of the tree, and collects + * the path of nodes to this leaf as a side-effect in the todo + * stack. + */ + private static IStringTreeNode leftmostLeaf(Deque todo, IStringTreeNode start) { + IStringTreeNode cur = start; - return cur; + while (cur.depth() > 1) { + todo.push(cur.right()); + cur = cur.left(); + } + + return cur; } - } } From f7d591e0314cb9a2597e59a16b83a8f7f7a7597c Mon Sep 17 00:00:00 2001 From: Davy Landman Date: Sun, 22 Sep 2024 16:53:14 +0200 Subject: [PATCH 14/14] Fixed indentation error --- .../io/usethesource/vallang/impl/primitive/StringValue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java index ab9b5190..89c11bc5 100644 --- a/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java +++ b/src/main/java/io/usethesource/vallang/impl/primitive/StringValue.java @@ -1327,7 +1327,7 @@ private static IStringTreeNode leftmostLeaf(Deque todo, IStringT } return cur; - } + } }