purescript / purescript-typelevel-prelude

Types and kinds for basic type-level programming
BSD 3-Clause "New" or "Revised" License
63 stars 21 forks source link

Add Mirror and Snoc type classes to Symbol #51

Open csicar opened 5 years ago

csicar commented 5 years ago

This PR adds Type-Classes for Mirroring Symbols ("Test" ~> "tseT") and Snoc ("Tes" "t" ~> "Test").

These Type-Classes can be useful when parsing Symbols to Numbers.

If this is to esoteric for the typelevel-prelude, feel free to reject the PR.

JordanMartinez commented 3 years ago

Could you rebase this on top of current master?

hdgarrood commented 3 years ago

I’m actually undecided on whether this ought to be in typelevel-prelude. It’s at a very high up place in the hierarchy so we should be quite choosy about what we put in it.

JordanMartinez commented 3 years ago

I’m actually undecided on whether this ought to be in typelevel-prelude. It’s at a very high up place in the hierarchy so we should be quite choosy about what we put in it.

Two questions. First, does the class and its instances look good, so that we should accept it (wherever it is we do put it). Second, where should it go?

Now that PolyKinds are supported, there are many other compile-time programming we can do. While the below question doesn't need to be answered before v0.14.0 comes out, it's been on my mind. We can now have a type-level Maybe, Either, and List, etc. as well as type-level functions for these types. Where should such data types and functions go in the ecosystem? Should typelevel-prelude be considered at the "bottom" of this type-level library ecosystem as prelude does for value-level libraries? Or does typelevel-prelude combine multiple libraries together?

hdgarrood commented 3 years ago

I don’t do enough type level programming to feel confident reviewing this, although I think Reverse might be a better name than Mirror.

Also, even if it does look good, that doesn’t necessarily mean it needs to live in core; it could live in some other library outside of core, or even in the project where it’s needed. I think there should be evidence of demand for something across multiple people and projects before we merge it in here, ideally.

Typelevel-prelude should be considered at the bottom of the hierarchy for type-level programming libraries because it only depends on prelude and type-equality (which is tiny), and because most other type-level libraries are likely to depend on it.

JordanMartinez commented 2 years ago

At the very least, this should include tests to verify it works as expected. I am curious how this would play with reifySymbol though, as I still don't get how one uses that function productively.

csicar commented 2 years ago

That's funny ^^ I had already written tests for this 2 years ago, but forgot to stage them in git. They are now pushed.

JordanMartinez commented 2 years ago

Can you add these two tests?

-- or something like this that verifies this is true
testReverseTwice :: forall a b. IsSymbol a => IsSymbol b => Reverse a b => Proxy a -> Boolean
testReverseTwice p = reflectSymbol p == reflectSymbol sameThing
  where
  sameThing = reverse (reverse p)

-- just confirming that more complex unicode works
testUnicode :: Proxy _
testUnicode = reverse (Proxy :: Proxy "pine🍎")
csicar commented 2 years ago

It would be nice to run this in quickcheck, but I cannot find a good way to use reifySymbol with typeclass-computations: reifySymbol qcSymbol testReverseTwice, but the type of

reifySymbol :: forall proxy r. String -> (forall sym. IsSymbol sym => proxy sym -> r) -> r

does not allow add additional constraints to the (forall sym. IsSymbol sym => proxy sym -> r argument, which would be necessary for Reverse.

In general adding additional constraints would not be sound. But because Reverse is implemented forall Symbols, it would be sound in the case of Reverse.

I.e. for Reverse such a function would be valid: canReverseAnySymbol :: ∀ a r. IsSymbol a => (∀ b. IsSymbol b => Reverse a b => r) -> r

LiamGoodacre commented 2 years ago

(I don't have any real opinion about whether this should be added, but...)

As i touched on above, i still think it'd make more sense to either:

The current implementation seems to have a bit too much going on regarding instance constraints.

I'm imagining something like:

class Reverse (i :: Symbol) (o :: Symbol) | i -> o
instance Reverse "" ""
else
instance ( Symbol.Cons h t l, Reverse t s, Symbol.Append s h r ) => Reverse l r

reverse :: forall i o proxy . Reverse i o => proxy i -> Proxy o
reverse _ = Proxy

--

example :: Proxy _ -- "fedcba"
example = reverse (Proxy :: _ "abcdef")

check :: Proxy _ -- True
check =
  Symbol.equals
    (reverse (reverse example))
    example

checkReflected :: Boolean
checkReflected =
  Symbol.reflectSymbol (reverse (reverse example))
  ==
  Symbol.reflectSymbol example
LiamGoodacre commented 2 years ago

As for Snoc, i feel that might be better as a Prim built-in like Cons is.