Skip to content

Commit 00f6dbc

Browse files
committed
Clean up buffer reads
1 parent 49427d8 commit 00f6dbc

File tree

4 files changed

+91
-7
lines changed

4 files changed

+91
-7
lines changed

parser-typechecker/src/Unison/Builtin.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -855,9 +855,9 @@ ioBuiltins =
855855
("IO.getSomeBytes.impl.v1", handle --> nat --> iof bytes),
856856
("IO.putBytes.impl.v3", handle --> bytes --> iof unit),
857857
("IO.getLine.impl.v1", handle --> iof text),
858-
("IO.fillBuf.impl.v1", forall1 "g" \g -> handle --> pinnedByteArrayt g --> iof nat),
858+
("IO.fillBuf.impl.v1", forall1 "g" \g -> handle --> pinnedByteArrayt g --> nat --> iof nat),
859859
("IO.putBuf.impl.v1", forall1 "g" \g -> handle --> pinnedByteArrayt g --> nat --> iof nat),
860-
("IO.getBufSome.impl.v1", forall1 "g" \g -> handle --> pinnedByteArrayt g --> iof nat),
860+
("IO.getBufSome.impl.v1", forall1 "g" \g -> handle --> pinnedByteArrayt g --> nat --> iof nat),
861861
("IO.systemTime.impl.v3", unit --> iof nat),
862862
("IO.systemTimeMicroseconds.v1", unit --> io int),
863863
("IO.getTempDirectory.impl.v3", unit --> iof text),

unison-runtime/src/Unison/Runtime/Foreign/Function.hs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ import UnliftIO qualified
207207

208208
withMutableByteArrayContents :: (PA.PrimBase m) => PA.MutableByteArray (PA.PrimState m) -> (Ptr Word8 -> m a) -> m a
209209
{-# INLINE withMutableByteArrayContents #-}
210-
withMutableByteArrayContents mba =
211-
PA.keepAlive (PA.mutableByteArrayContents mba)
210+
withMutableByteArrayContents mba f =
211+
PA.keepAlive mba (f . PA.mutableByteArrayContents)
212212

213213
-- foreignCall is explicitly NOINLINE'd because it's a _huge_ chunk of code and negatively affects code caching.
214214
-- Because we're not inlining it, we need a wrapper using an explicitly unboxed Stack so we don't block the
@@ -286,13 +286,21 @@ foreignCallHelper = \case
286286
\(h, n) -> Bytes.fromArray <$> hGetSome h n
287287
IO_putBytes_impl_v3 -> mkForeignIOF $ \(h, bs) -> hPut h (Bytes.toArray bs)
288288
-- TODO: Use `PA.withMutableByteArrayContents` here once we have Data.Primitive v9.
289-
IO_fillBuf_impl_v1 -> mkForeignIOF $ \(h, arr) -> let !sz = PA.sizeofMutableByteArray arr in withMutableByteArrayContents arr (\ptr -> hGetBuf h ptr sz)
289+
IO_fillBuf_impl_v1 -> mkForeignIOF $ \(h, arr, n) -> do
290+
r <- checkBoundsPrim "IO.fillBuf.impl.v1" (PA.sizeofMutableByteArray arr) n 0 . pure $ Right ()
291+
case r of
292+
Left (F.Failure _ err _) -> ioError (userError (Util.Text.unpack err))
293+
Right _ -> withMutableByteArrayContents arr (\ptr -> hGetBuf h ptr (fromIntegral n))
290294
IO_putBuf_impl_v1 -> mkForeignIOF $ \(h, arr, n) -> do
291295
r <- checkBoundsPrim "IO.putBuf.impl.v1" (PA.sizeofMutableByteArray arr) n 0 . pure $ Right ()
292296
case r of
293297
Left (F.Failure _ err _) -> ioError (userError (Util.Text.unpack err))
294-
Right _ -> hPutBuf h (PA.mutableByteArrayContents arr) (fromIntegral n)
295-
IO_getBufSome_impl_v1 -> mkForeignIOF $ \(h, arr) -> let !sz = PA.sizeofMutableByteArray arr in withMutableByteArrayContents arr (\ptr -> hGetBufSome h ptr sz)
298+
Right _ -> withMutableByteArrayContents arr (\ptr -> hPutBuf h ptr (fromIntegral n))
299+
IO_getBufSome_impl_v1 -> mkForeignIOF $ \(h, arr, n) -> do
300+
r <- checkBoundsPrim "IO.getBufSome.impl.v1" (PA.sizeofMutableByteArray arr) n 0 . pure $ Right ()
301+
case r of
302+
Left (F.Failure _ err _) -> ioError (userError (Util.Text.unpack err))
303+
Right _ -> withMutableByteArrayContents arr (\ptr -> hGetBufSome h ptr (fromIntegral n))
296304
IO_systemTime_impl_v3 -> mkForeignIOF $
297305
\() -> getPOSIXTime
298306
IO_systemTimeMicroseconds_v1 -> mkForeign $

unison-src/transcripts-using-base/all-base-hashes.output.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1725,6 +1725,7 @@ This transcript is intended to make visible accidental changes to the hashing al
17251725
477. -- ##IO.fillBuf.impl.v1
17261726
builtin.io2.IO.fillBuf.impl : Handle
17271727
-> PinnedByteArray g
1728+
-> Nat
17281729
->{IO} Either Failure Nat
17291730
17301731
478. -- ##IO.forkComp.v2
@@ -1741,6 +1742,7 @@ This transcript is intended to make visible accidental changes to the hashing al
17411742
481. -- ##IO.getBufSome.impl.v1
17421743
builtin.io2.IO.getBufSome.impl : Handle
17431744
-> PinnedByteArray g
1745+
-> Nat
17441746
->{IO} Either Failure Nat
17451747
17461748
482. -- ##IO.getBytes.impl.v3

unison-src/transcripts/idempotent/io.md

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,80 @@ testGetSomeBytes _ =
261261
Tip: Use view 1 to view the source of a test.
262262
```
263263

264+
### Reading and writing via buffers
265+
266+
Tests:
267+
268+
- fillBuf
269+
- putBuf
270+
- getBufSome
271+
272+
``` unison
273+
testFillBuf : '{io2.IO} [Result]
274+
testFillBuf = do
275+
test = 'let
276+
tempDir = (newTempDir "getSomeBytes")
277+
fooFile = tempDir ++ "/foo"
278+
279+
testData = "0123456789"
280+
testSize = size testData
281+
282+
wbuf = IO.pinnedByteArray 10
283+
arr = PinnedByteArray.cast wbuf
284+
dataBytes = ImmutableByteArray.fromBytes (toUtf8 testData)
285+
ImmutableByteArray.copyTo! arr 0 dataBytes 0 10
286+
287+
-- write testData to a temporary file
288+
fooWrite = openFile fooFile Write
289+
_ = putBuf.impl fooWrite wbuf 10
290+
closeFile fooWrite
291+
check "file should be closed" (not (isFileOpen fooWrite))
292+
293+
-- reopen for reading back the data in chunks
294+
fooRead = openFile fooFile Read
295+
296+
rbuf = IO.pinnedByteArray 10
297+
rarr = PinnedByteArray.cast rbuf
298+
_ = fillBuf.impl fooRead rbuf 4
299+
vec = ImmutableByteArray.toBytes (MutableByteArray.freeze rarr 0 4) 0 4
300+
check "should be able to read 4 bytes" (vec == (toUtf8 testData |> take 4))
301+
302+
_ = getBufSome.impl fooRead rbuf 4
303+
vec2 = ImmutableByteArray.toBytes (MutableByteArray.freeze rarr 0 4) 0 4
304+
check "should be able to read next 4 bytes" (vec2 == (toUtf8 testData |> drop 4 |> take 4))
305+
306+
runTest test
307+
```
308+
309+
``` ucm :added-by-ucm
310+
Loading changes detected in scratch.u.
311+
312+
+ testFillBuf : '{IO} [Result]
313+
314+
Run `update` to apply these changes to your codebase.
315+
```
316+
317+
``` ucm
318+
> add
319+
320+
Okay, I'm searching the branch for code that needs to be
321+
updated...
322+
323+
Done.
324+
325+
> io.test testFillBuf
326+
327+
New test results:
328+
329+
1. testFillBuf ◉ file should be closed
330+
◉ should be able to read 4 bytes
331+
◉ should be able to read next 4 bytes
332+
333+
✅ 3 test(s) passing
334+
335+
Tip: Use view 1 to view the source of a test.
336+
```
337+
264338
### Seeking in open files
265339

266340
Tests:

0 commit comments

Comments
 (0)