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

Pattern guards are not a sufficient replacement for view patterns #174

Open
expipiplus1 opened this issue Jan 13, 2023 · 7 comments
Open

Comments

@expipiplus1
Copy link

The implementation for desugaring view patterns is dsPat (ViewP _ _) = dsPat (ViewP _ _) = fail "View patterns are not supported in th-desugar. Use pattern guards instead."

It's not possible to use pattern guards in pattern synonyms, so desugaring the following will fail:

  x :: [DDec] <-
    desugar
      [ PatSynD
          (mkName "pat")
          (RecordPatSyn [])
          (ExplBidir [])
          ( ViewP
              (VarE (mkName "a"))
              (TupP [])
          )
      ]

And of course it's not possible to construct such pattern synonyms!

There's definitely a complexity/usefulness tradeoff to be struck; although I'd like to be able to use view patterns with th-desugar I would understand if this is closed with something like "not worth the complexity".

@RyanGlScott
Copy link
Collaborator

This is most likely a consequence of two historical coincidences:

  1. This code was first written before pattern synonyms were a thing. At the time, any use of view patterns could be straightforwardly converted to pattern guards (which then can be desugared to case expressions), so view patterns were excluded from th-desugar's AST to keep things simpler.
  2. th-desugar was originally forked from the singletons library. There is no reasonable way to make view patterns work in the context of singletons, so that was another reason to keep view patterns out of the th-desugar AST.

Now that pattern synonyms are a thing (and that we have th-desugar users besides singletons), we should revisit these assumptions. I agree that view patterns are quite essential for defining many kinds of pattern synonyms, and there really isn't a viable alternative to them—at least, not in this particular spot. Some possible ways to handle this:

  1. Add a DViewP :: DExp -> DPat -> DPat data constructor to DPat. This would allow defining view patterns, but now all downstream consumers of th-desugar will be forced to reckon with view patterns in any place where DPats can occur.

  2. Leave DPat alone, and instead add a new DPatSynPat data type:

    data DPatSynPat
      = DPatSynViewPat DExp DPatSynViewPat
      | DPatSynPat DPat

    Then change the DPat field in DPatSynD to a DPatSynPat, leaving all other uses of DPat alone. This would limit the scope of the change to just pattern synonym declarations. The downside is that this would only be able to express non-"prenex" uses of view patterns. For instance, this would permit pattern P1 x <- (f -> g -> x), but not pattern P2 x y = (f -> x, g -> y).

  3. Introduce a simpler variant of DPatSynPat:

    data DPatSynPat
      = DPatSynViewPat DExp DPat
      | DPatSynPat DPat

    If we encounter nested uses of view patterns, introduce a fresh auxiliary function that collapses all the view patterns into a single one. For instance, desugar pattern P2 x y = (f -> x, g -> y) into something like this:

    pattern P2 x y <- (_aux -> (x, y))
    
    _aux (x', y') = (f x', g y')

    The downside to this approach is that we would need to pollute the top-level namespace with these _aux functions.

Perhaps there are other solutions I haven't thought of as well. What are your thoughts on this, @goldfirere?

@expipiplus1
Copy link
Author

Why couldn't option 3 just define _aux inline?

@RyanGlScott
Copy link
Collaborator

Ah, great point! For some reason, I had it in my mind that the thing to the left of the -> had to be a bare function name, but we could just as well use an arbitrary expression, e.g.,

pattern P2 x y <- ((\(x', y') -> (f x', g y')) -> (x, y))

That should work pretty nicely, I think. I like it.

@RyanGlScott
Copy link
Collaborator

It occurs to me that this approach could also resolve another restriction that th-desugar imposes on pattern synonyms. Currently, you can't desugar a pattern synonym where the right-hand side contains an as-pattern, e.g., pattern P x y z <- x@(y, z). (This is a GHC feature that was added after this proposal.) If we added support for "top-level" view patterns as described above, however, then this could be desugared as:

pattern P x y z <- (\x' -> case x' of (y', z') -> (x', y', z')) (x, y, z)

@expipiplus1
Copy link
Author

It occurs to me that this approach could also resolve another restriction that th-desugar imposes on pattern synonyms. Currently, you can't desugar a pattern synonym where the right-hand side contains an as-pattern...

That's easy to implement using dsPatX however that leads to some uncomfortable unpacking/repacking...

pattern P x a b <- (\arg_2_0 -> case arg_2_0 of
                                    Foo a b -> GHC.Tuple.(,,) a b (Foo a b) -> GHC.Tuple.(,,) a b x)

@expipiplus1
Copy link
Author

ok, there's a (unneatened) PR for this.

aside: It's a shame that th-desugar doesn't use lambdacase to replace DCaseE and DLamE; lambdacase seems like the more "distilled" construct. I guess the ship has sailed on this though..

@RyanGlScott
Copy link
Collaborator

It's a shame that th-desugar doesn't use lambdacase to replace DCaseE and DLamE; lambdacase seems like the more "distilled" construct. I guess the ship has sailed on this though..

For what it's worth, I encountered another situation in which it would be nice to use \cases rather than DCaseE/DLamE, so I've opened #210 to track the using \cases within th-desugar. This would require a fair bit of work to do, but the end result is that we may be able to avoid some of the unsightly tuple unpacking/packing seen in #174 (comment).

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

No branches or pull requests

2 participants