Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The PrimUnlifted ShortText instance is unsafe #22

Open
treeowl opened this issue Oct 20, 2020 · 0 comments · May be fixed by #23
Open

The PrimUnlifted ShortText instance is unsafe #22

treeowl opened this issue Oct 20, 2020 · 0 comments · May be fixed by #23

Comments

@treeowl
Copy link
Contributor

treeowl commented Oct 20, 2020

The PrimUnlifted ShortText instance uses fromShortByteStringUnsafe, which is considered Unsafe. I don't know just how wrong things can go if the bytestring isn't properly encoded, but based on the liberal "Unsafe!" warnings around FFI in text-short, I'd be worried.

Here's what I suggest: define an unlifted newtype around ByteArray# in an Unsafe module. Use that as the definition of Unlifted ShortText. Mark Data.Primitive.Unlifted.Class as Trustworthy. Users who just want to be able to make unlifted containers of ShortText won't have to change anything. Those who actually need to mess with the unlifted representation can import the unsafe module and do what they like.

treeowl added a commit to treeowl/primitive-unlifted that referenced this issue Oct 20, 2020
* Use a newtype to hide the unrestricted `ByteArray`
  underneath.

* Mark `Data.Primitive.Unlifted.Class` as `Trustworthy`. TODO:
  Discuss whether it's okay to do that without similarly changing
  the `PrimArray` instance.

Closes haskell-primitive#22
@treeowl treeowl linked a pull request Oct 20, 2020 that will close this issue
treeowl added a commit to treeowl/primitive-unlifted that referenced this issue Oct 20, 2020
* Use a newtype to hide the unrestricted `ByteArray`
  underneath.

* Mark `Data.Primitive.Unlifted.Class` as `Trustworthy`. TODO:
  Discuss whether it's okay to do that without similarly changing
  the `PrimArray` instance.

Closes haskell-primitive#22
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 a pull request may close this issue.

1 participant