Skip to content

Commit

Permalink
fix incorrect resizing logic in bytes struct
Browse files Browse the repository at this point in the history
  • Loading branch information
thatstoasty committed Mar 25, 2024
1 parent 88e2209 commit 5ed54ac
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 14 deletions.
2 changes: 2 additions & 0 deletions gojo/bufio/bufio.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,8 @@ struct Writer[W: io.Writer](
self.err = error
return error

# Reset the buffer
self.buf = Bytes(size=self.buf.size())
self.bytes_written = 0
return None

Expand Down
17 changes: 14 additions & 3 deletions gojo/builtins/_bytes.mojo
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from time import now
from .errors import panic


Expand Down Expand Up @@ -89,11 +88,17 @@ struct Bytes(Stringable, Sized, CollectionElement):
return new_bytes

fn __setitem__(inout self, index: Int, value: Int8):
if index >= len(self._vector):
panic("Bytes.__setitem__: Tried setting index out of range. Vector length is " + str(len(self._vector)) + " but tried to set index " + str(index) + ".")

self._vector[index] = value
if index >= self.write_position:
self.write_position = index + 1

fn __setitem__(inout self, index: Int, value: Self):
if index >= len(self._vector):
panic("Bytes.__setitem__: Tried setting index out of range. Vector length is " + str(len(self._vector)) + " but tried to set index " + str(index) + ".")

self._vector[index] = value[0]
if index >= self.write_position:
self.write_position = index + 1
Expand Down Expand Up @@ -139,6 +144,12 @@ struct Bytes(Stringable, Sized, CollectionElement):

fn __repr__(self) -> String:
return self.__str__()

fn __contains__(self, item: Int8) -> Bool:
for i in range(len(self)):
if self[i] == item:
return True
return False

fn append(inout self, value: Byte):
"""Appends the value to the end of the Bytes.
Expand All @@ -147,7 +158,7 @@ struct Bytes(Stringable, Sized, CollectionElement):
value: The value to append.
"""
# If the vector is full, resize it.
if self.write_position == self.size():
if self.write_position >= self.size():
self.resize(self.size() * 2)
self[self.write_position] = value

Expand Down Expand Up @@ -239,4 +250,4 @@ struct Bytes(Stringable, Sized, CollectionElement):
"""
var new_bytes = self.copy()._vector
new_bytes.append(0)
return new_bytes
return new_bytes
6 changes: 5 additions & 1 deletion gojo/builtins/attributes.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@ fn copy(inout target: Bytes, source: Bytes, start: Int = 0) -> Int:
"""
var count = 0

# If trying to set a value outside of the bounds of the buffer, use append instead. It will resize the buffer before setting the value.
for i in range(len(source)):
target[i + start] = source[i]
if i + start < target.size():
target[i + start] = source[i]
else:
target.append(source[i])
count += 1

return count
Expand Down
20 changes: 10 additions & 10 deletions gojo/bytes/buffer.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ struct Buffer(
It returns the index where bytes should be written and whether it succeeded."""
var buffer_already_used = len(self.buf)

if n <= cap(self.buf) - buffer_already_used:
if n <= self.buf.size() - buffer_already_used:
# FIXME: It seems like reslicing in go can extend the length of the slice. Doens't work like that for my get slice impl.
# Instead, just add bytes of len(n) to the end of the buffer for now.
# self.buf = self.buf[: l + n]
Expand All @@ -159,7 +159,7 @@ struct Buffer(
"""Grows the buffer to guarantee space for n more bytes.
It returns the index where bytes should be written.
If the buffer can't grow it will panic with ERR_TOO_LARGE."""
var write_at: Int = self.buf.size()
var write_at: Int = len(self.buf)
# If buffer is empty, reset to recover space.
if write_at == 0 and self.off != 0:
self.reset()
Expand All @@ -182,15 +182,14 @@ struct Buffer(
# slice. We only need m+n <= c to slide, but
# we instead var capacity get twice as large so we
# don't spend all our time copying.

_ = copy(self.buf, self.buf[self.off :])
elif c > MAX_INT - c - n:
panic(ERR_TOO_LARGE)
else:
# Add self.off to account for self.buf[:self.off] being sliced off the front.
var sl = self.buf[self.off :]
self.buf = self.grow_slice(sl, self.off + n)
# self.buf = sl
# TODO: Commented out this branch because growing the slice here and then at the end is redundant?
# else:
# # Add self.off to account for self.buf[:self.off] being sliced off the front.
# # var sl = self.buf[self.off :]
# # self.buf = self.grow_slice(sl, self.off + n)

# Restore self.off and len(self.buf).
self.off = 0
Expand Down Expand Up @@ -231,7 +230,8 @@ struct Buffer(
if not ok:
write_at = self.grow(len(src))

return Result(copy(self.buf, src, write_at), None)
var bytes_written = copy(self.buf, src, write_at)
return Result(bytes_written, None)

fn write_string(inout self, src: String) -> Result[Int]:
"""Appends the contents of s to the buffer, growing the buffer as
Expand Down Expand Up @@ -302,7 +302,7 @@ struct Buffer(
# we could rely purely on append to determine the growth rate.
c = 2 * cap(b)

var resized_buffer = Bytes(c)
var resized_buffer = Bytes(size=c)
_ = copy(resized_buffer, b)
# var b2: Bytes = Bytes()
# b2._vector.reserve(c)
Expand Down
22 changes: 22 additions & 0 deletions tests/test_bufio.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,27 @@ fn test_write() raises:
test.assert_equal(str(writer.writer), "0123456789")


fn test_several_writes() raises:
var test = MojoTest("Testing several bufio.Writer.write")

# Create a new Bytes Buffer Writer and use it to create the buffered Writer
var buf = buffer.new_buffer()
var writer = Writer(buf)

# Write the content from src to the buffered writer's internal buffer and flush it to the Bytes Buffer Writer.
var src = Bytes("0123456789")
var result = Result(0)
for i in range(500):
result = writer.write(src)
_ = writer.flush()

test.assert_equal(result.value, 10)
test.assert_equal(len(writer.writer), 5000)
var text = str(writer.writer)
test.assert_equal(text[0], "0")
test.assert_equal(text[4999], "9")


fn test_write_byte() raises:
var test = MojoTest("Testing bufio.Writer.write_byte")

Expand Down Expand Up @@ -193,6 +214,7 @@ fn main() raises:
test_peek()
test_discard()
test_write()
test_several_writes()
test_write_byte()
test_write_string()
test_read_from()

0 comments on commit 5ed54ac

Please sign in to comment.