Skip to content

Conversation

@neilmayhew
Copy link
Contributor

@neilmayhew neilmayhew commented Jan 24, 2026

Description

Move these instances closer to their types. This enables us to write better tests for cardano-base libraries.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated
  • All visible changes are prepended to the latest section of a CHANGELOG.md for the affected packages.
    New section is never added with the code changes. (See RELEASING.md)
  • When applicable, versions are updated in .cabal and CHANGELOG.md files according to the
    versioning process.
  • The version bounds in .cabal files for all affected packages are updated.
    If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)
  • Self-reviewed the diff

Comment on lines +25 to +28
byteArrayToByteString ba@(ByteArray ba#)
| isByteArrayPinned ba =
BS.fromForeignPtr (pinnedByteArrayToForeignPtr ba#) 0 (sizeofByteArray ba)
| otherwise = SBS.fromShort (byteArrayToShortByteString ba)
Copy link
Collaborator

@lehins lehins Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fromShort already performs the check for pinnedness: https://hackage-content.haskell.org/package/bytestring-0.11.5.4/docs/src/Data.ByteString.Short.Internal.html#fromShort

Suggested change
byteArrayToByteString ba@(ByteArray ba#)
| isByteArrayPinned ba =
BS.fromForeignPtr (pinnedByteArrayToForeignPtr ba#) 0 (sizeofByteArray ba)
| otherwise = SBS.fromShort (byteArrayToShortByteString ba)
byteArrayToByteString ba = SBS.fromShort (byteArrayToShortByteString ba)

FTR. It didn't used to before in older versions of bytestring, hence the original implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants