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

Move pattern guards to CoreFn #4496

Open
natefaubion opened this issue Aug 2, 2023 · 3 comments
Open

Move pattern guards to CoreFn #4496

natefaubion opened this issue Aug 2, 2023 · 3 comments

Comments

@natefaubion
Copy link
Contributor

natefaubion commented Aug 2, 2023

Summary

Move pattern guards to CoreFn, and desugar them as part of codegen.

Motivation

backend-optimizer optimizes pattern-matching decision trees, but pattern guard desugaring commits too early to a particular control flow through it's CPS translation. If pattern guards were added to CoreFn, we would have more opportunity to analyze these cases and potentially add them to the existing matrix for additional sharing.

Proposal

Currently in CoreFn, a Guard for a case branch is an alias for Expr (ie, a single boolean expression).

-- |
-- A guard is just a boolean-valued expression that appears alongside a set of binders
--
type Guard a = Expr a

This could be changed to something like:

-- |
-- A guard may bind an expression, or implicitly guard against a boolean expression
--
type Guard a = [(Maybe (Binder a), Expr a)]

-- |
-- An alternative in a case statement
--
data CaseAlternative a = CaseAlternative
  { -- |
    -- A collection of binders with which to match the inputs
    --
    caseAlternativeBinders :: [Binder a]
    -- |
    -- The result expression or a collect of guarded expressions
    --
  , caseAlternativeResult :: Either [(Guard a, Expr a)] (Expr a)
  } deriving (Eq, Ord, Show)

Alternatively, we could desugar all guards to a true <- ... pattern guard and ditch the Maybe.

@rhendric
Copy link
Member

rhendric commented Aug 2, 2023

Currently, caseAlternativeResult is Either [(Guard a, Expr a)] (Expr a), where the left side covers boolExpr | rhs ; anotherBoolExpr | differentRhs .... How would that case be represented with this change?

@natefaubion
Copy link
Contributor Author

natefaubion commented Aug 2, 2023

Oops! You're right, I missed the outer [ ... ]. I've updated it to be NonEmpty.

Edit: Doh! That's not right either. I'll refactor this, but I think the idea still stands 😅

@natefaubion
Copy link
Contributor Author

I've reverted the change to caseAlternativeResult. CoreFn is a bit off in general in that it allows empty lists where there shouldn't be, so I think just changing Guard is fine.

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

No branches or pull requests

2 participants