Skip to content

Remove some usages of unsafeCoerce #184

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

Merged
merged 2 commits into from
Nov 25, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/Data/Array.purs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ import Control.Alternative (class Alternative)
import Control.Lazy (class Lazy, defer)
import Control.Monad.Rec.Class (class MonadRec, Step(..), tailRecM2)
import Control.Monad.ST as ST
import Data.Array.NonEmpty.Internal (NonEmptyArray)
import Data.Array.NonEmpty.Internal (NonEmptyArray(..))
import Data.Array.ST as STA
import Data.Array.ST.Iterator as STAI
import Data.Foldable (class Foldable, foldl, foldr, traverse_)
Expand All @@ -134,7 +134,6 @@ import Data.Traversable (sequence, traverse)
import Data.Tuple (Tuple(..), fst, snd)
import Data.Unfoldable (class Unfoldable, unfoldr)
import Partial.Unsafe (unsafePartial)
import Unsafe.Coerce (unsafeCoerce)

-- | Convert an `Array` into an `Unfoldable` structure.
toUnfoldable :: forall f. Unfoldable f => Array ~> f
Expand Down Expand Up @@ -895,7 +894,7 @@ groupBy op xs =
_ <- STA.push x sub
STAI.pushWhile (op x) iter sub
grp <- STA.unsafeFreeze sub
STA.push ((unsafeCoerce :: Array ~> NonEmptyArray) grp) result
STA.push (NonEmptyArray grp) result
STA.unsafeFreeze result

-- | Remove the duplicates from an array, creating a new array.
Expand Down
9 changes: 5 additions & 4 deletions src/Data/Array/NonEmpty.purs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Data.Array.NonEmpty
( module Data.Array.NonEmpty.Internal
( module Internal
, fromArray
, fromNonEmpty
, toArray
Expand Down Expand Up @@ -101,7 +101,8 @@ import Control.Alternative (class Alternative)
import Control.Lazy (class Lazy)
import Control.Monad.Rec.Class (class MonadRec)
import Data.Array as A
import Data.Array.NonEmpty.Internal (NonEmptyArray)
import Data.Array.NonEmpty.Internal (NonEmptyArray(..))
import Data.Array.NonEmpty.Internal (NonEmptyArray) as Internal
import Data.Bifunctor (bimap)
import Data.Foldable (class Foldable)
import Data.Maybe (Maybe(..), fromJust)
Expand Down Expand Up @@ -139,7 +140,7 @@ fromArray xs

-- | INTERNAL
unsafeFromArray :: forall a. Array a -> NonEmptyArray a
unsafeFromArray = unsafeCoerce
unsafeFromArray = NonEmptyArray

unsafeFromArrayF :: forall f a. f (Array a) -> f (NonEmptyArray a)
unsafeFromArrayF = unsafeCoerce
Expand All @@ -148,7 +149,7 @@ fromNonEmpty :: forall a. NonEmpty Array a -> NonEmptyArray a
fromNonEmpty (x :| xs) = cons' x xs

toArray :: forall a. NonEmptyArray a -> Array a
toArray = unsafeCoerce
toArray (NonEmptyArray xs) = xs

toNonEmpty :: forall a. NonEmptyArray a -> NonEmpty Array a
toNonEmpty = uncons >>> \{head: x, tail: xs} -> x :| xs
Expand Down
12 changes: 11 additions & 1 deletion src/Data/Array/NonEmpty/Internal.purs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
module Data.Array.NonEmpty.Internal (NonEmptyArray) where
-- | This module exports the `NonEmptyArray` constructor, which can be
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be worth discouraging this module’s use a bit more, by adding a disclaimer that this module is not intended for public use and its API may change at any time, and a breaking change won’t be reflected in the version number?

Copy link
Member Author

@kl0tl kl0tl Nov 20, 2020

Choose a reason for hiding this comment

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

What do you think of this, with some bold all caps and newlines to let the disclaimer breath:

Capture d’écran 2020-11-20 à 09 18 48

It could be nice of Pursuit to display a warning on internal modules, so we can have some CSS on top and we don’t have to synchronize them across packages.

-- | overlooked when searching for issues in one's code. See the
-- | constructor's documentation for more information.
module Data.Array.NonEmpty.Internal (NonEmptyArray(..)) where

import Prelude

Expand All @@ -14,6 +17,13 @@ import Data.Traversable (class Traversable)
import Data.TraversableWithIndex (class TraversableWithIndex)
import Data.Unfoldable1 (class Unfoldable1)

-- | An array that is known not to be empty.
-- |
-- | You can use the constructor to create a `NonEmptyArray` that isn't
-- | non-empty, breaking the guarantee behind this newtype. It is
-- | provided as an escape hatch mainly for the `Data.Array.NonEmpty`
-- | and `Data.Array` modules. Use this at your own risk when you know
-- | what you are doing.
newtype NonEmptyArray a = NonEmptyArray (Array a)

instance showNonEmptyArray :: Show a => Show (NonEmptyArray a) where
Expand Down