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

Support discriminated unions in the type system #244

Closed
2 tasks done
bugarela opened this issue Sep 28, 2022 · 2 comments · Fixed by #1232
Closed
2 tasks done

Support discriminated unions in the type system #244

bugarela opened this issue Sep 28, 2022 · 2 comments · Fixed by #1232
Assignees
Labels
effort-hard Takes >= 5 days (probably requires issue refactor) impact-medium Medium impact typechecker Type checker for Quint

Comments

@bugarela
Copy link
Collaborator

bugarela commented Sep 28, 2022

We need to implement variants and use them to type discriminated unions and their operators (match) in the type system.

@bugarela bugarela self-assigned this Sep 28, 2022
@bugarela bugarela added the typechecker Type checker for Quint label Sep 28, 2022
@bugarela bugarela added this to the T1.3.2 Type System milestone Sep 28, 2022
@konnov konnov added impact-medium Medium impact effort-hard Takes >= 5 days (probably requires issue refactor) labels Nov 29, 2022
@shonfeder
Copy link
Contributor

shonfeder commented Dec 2, 2022

Leaving a note here on (what looks to me like) a somewhat unfortunate variation from the representation of discriminated unions in the TlaType1 representation:

afaiu, TlaType1 follows the approach to extensible variants described in https://homepage.cs.uiowa.edu/~jgmorrs/pubs/morris-popl2019-rows.pdf, in (extensible) variants obtained through the sum of a single record. But in TNT we are representing unions as a set of tagged rows.

E.g.,

https://github.com/informalsystems/tnt/blob/a0ac34896f5205e3b8d9894ecec1a556df6cf303/tntc/test/types/parser.test.ts#L82-L109

If we were following the "Abstracting Extensible Data Types" and TlaType1, we would have instead something like

    \\ Maybe we'd have a different notation for the union tags
    const type = parseType('| a(int) | b(bool)')

    assert.isTrue(type.isRight())
    type.map(value => assert.deepEqual(value, {
      kind: 'union',
      row: {
        kind: 'row',
        fields: [
          { fieldName: 'a', fieldType: { kind: 'int', id: 1n } },
          { fieldName: 'b', fieldType: { kind: 'bool', id: 2n } }],
        other: { kind: 'empty' },
      },
      id: 3n,
    }
    ))
  })

This seems much nicer to me on every level:

  • The input syntax is clear and more flexible
  • The implementation is simpler
  • It seems to be more elegant theoretically, in allowing rows to express both records and variants by application of product or sum operations.

@konnov
Copy link
Contributor

konnov commented Jan 19, 2023

I also like the approach implemented in TlaType1 currently, though it may have some quirks we have not had time to think about.

@konnov konnov added the W8 label Feb 16, 2023
thpani added a commit that referenced this issue Jul 26, 2023
This fails for lack of support of discriminated unions (#244)
thpani added a commit that referenced this issue Jul 26, 2023
These are failing because they use discriminted unions: #244
@bugarela bugarela assigned shonfeder and unassigned bugarela Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-hard Takes >= 5 days (probably requires issue refactor) impact-medium Medium impact typechecker Type checker for Quint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants