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

Add syntax for constructing and eliminating sum type values #1093

Closed
wants to merge 17 commits into from

Conversation

shonfeder
Copy link
Contributor

@shonfeder shonfeder commented Aug 5, 2023

Closes #1092

This followup to #1089 adds two more additions to our syntax for sum types, following the design proposed in #1062.

New additions to the IR:

  • QuintMatch, representing match expressions for eliminating variants of a sum type.
  • QuintVaraint, representing a value of a sum type. This just pairs a label with a value.

Additionally, as per RFC001, parsing a sum-type declaration generates constructors for each variant.

A number of minor fixes needed to be made along the way.

Key concepts

Match expressions are represented as associations of variant labels and lambdas that can be applied to the argument of a variant. E.g.,

match x {A => e1 | B(v) => e2 }

is represented as (in pseudo code)

{ expr: x,
, cases: [ ('A', (()) => e1)
         , ('B', (v) => e2)
         ]
}

A variant value is basically just a tuple pairing a label with the associated expression (but uses an object to match the rest or our IR).

Reviewing

  • Reviewing by commit, and skipping the generated stuff, is probably best!
  • This is currently branched off of Fix non-exhaustive switches #1092, and it will be a lot smaller once that is merged in.
  • Tests added for any new code
  • [-] Documentation added for any new functionality
  • [-] Entries added to the respective CHANGELOG.md for any new functionality
  • [-] Feature table on README.md updated for any listed functionality

@shonfeder shonfeder force-pushed the sum-types/1082/match-and-constructor-syntax branch from 2637900 to 221d88a Compare August 5, 2023 04:27
Shon Feder added 12 commits August 7, 2023 10:17
These changes address the comments from
#1088

I had fixed all of these, but then merged the PR without pushing my
fixes.
This tests a bunch of behavior the naive approach was missing, and
scales much better for more than a single definition.
Fixing a commented out test condition and improving naming
This killed some time for me cause I couldn't figure out where the
fixtures where going wrong! Turned out, I just need to rebuild :D
@shonfeder shonfeder force-pushed the sum-types/1082/match-and-constructor-syntax branch from 221d88a to 41d9b61 Compare August 7, 2023 14:18
quint/src/flattening.ts Outdated Show resolved Hide resolved
quint/src/parsing/ToIrListener.ts Outdated Show resolved Hide resolved
quint/src/parsing/ToIrListener.ts Outdated Show resolved Hide resolved
quint/src/parsing/ToIrListener.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@konnov konnov left a comment

Choose a reason for hiding this comment

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

Left a comment on the extension of QuintEx, which IMO does not align with the current IR design. In the future, we may want to introduce high-level IR, e.g., HighQuintEx that expresses the special operators of QuintEx via case distinction.

Comment on lines +251 to +274
export type MatchCase = {
/** the variant label to match */
label: string
/** the lamba to apply to the variant's wrapped value */
elim: QuintLambda
}

export interface QuintMatch extends WithId {
kind: 'match'
/** the variant expression to match */
expr: QuintEx
/** the cases to match against */
cases: MatchCase[]
}

/** A variant value inhabiting a sum type */
export interface QuintVariant extends WithId {
kind: 'variant'
/** the varian't tag, e.g., the `A` in `A(n)` */
label: string
/** the expression injected into the sum type */
expr?: QuintEx
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We probably have different opinions on how the Quint IR should look like. So far, we have managed to keep the number of cases in QuintEx at minimum. This required us to introduce "special" low-level operators like Rec(...) and field(...) instead of having distinct cases in the IR. Although the normal users should not use these operators, they simplify IR manipulation at large.

Just to stress here, I am not arguing that a high-level IR is worse than a low-level IR, it is just that the current IR is minimal and low-level.

Given the above reasoning, I am proposing to introduce two new "special" operators instead of introducing two new cases of QuintEx, namely:

// Construct a variant of a sum type, where `label` carries the name of the label to be used in the type.
variant(label: str, payload: a): { label(a) | b }
// Match an expression with one of the labels `label_1`, ..., `label_n`.
// Process each case with the operators `f_1`, ..., `f_n`, respectively.
match(expr: a, label_1: str, f_1: (...) => ..., ..., label_n: str, f_n: (...) => ...): b

Since we are already processing records and tuples in a special way, I don't see why we could not do the same with sum types. The only big difference here is that match is a higher-order operator.

Copy link
Contributor Author

@shonfeder shonfeder Aug 21, 2023

Choose a reason for hiding this comment

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

In general:

we have managed to keep the number of cases in QuintEx at minimum. This required us to introduce "special" low-level operators like Rec(...) and field(...) instead of having distinct cases in the IR. Although the normal users should not use these operators, they simplify IR manipulation at large.

But what is the gain here? I've enumerated some of the costs here:

In the case of these kind of special constructs, any savings on the cases on QuintEx are paid for directly in the number of cases in the operator application logic, only we lose structure along the way and up having to perform unnecessarily validations and computations to reconstruct that lost data.

This PR is meant in part to show the benefit of dropping these exotic operator in favor a richer and more helpful representation in the IR.

But if you could help me understand where we actually see simplifications instead of complexity, that could help me move past my aversion here.

In particular:

Wouldn't adding the varaiant operator require support for open rows in sum-types, which we agreed (as per your preference) we did not want to expose in #1062?

We will need special cases for most operator applications to handle match, but we will lose the typing info and structure provided here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I talked thru the pros and cons of our current approach vs. a richer representation in the ir with @bugarela. I’m still of the opinion that the current approach is worse for most Americans d the most important purposes than a more structured representation, but I see the trade offs clearer now, and am fine with implementing this using the exotic operators. I’ll make a new pr, since this will be a very different approach and there are lots of merge conflicts here anyhow.

@shonfeder shonfeder closed this Oct 5, 2023
@shonfeder shonfeder deleted the sum-types/1082/match-and-constructor-syntax branch October 5, 2023 15:24
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 this pull request may close these issues.

2 participants