Skip to content

feat!: saturated constructors stage 4#961

Merged
brprice merged 12 commits intomainfrom
brprice/saturated-constructors-4
May 11, 2023
Merged

feat!: saturated constructors stage 4#961
brprice merged 12 commits intomainfrom
brprice/saturated-constructors-4

Conversation

@brprice
Copy link
Copy Markdown
Contributor

@brprice brprice commented Apr 24, 2023

This PR changes the typechecking treatment of constructors to be checkable (rather than synthesisable).

@brprice brprice changed the base branch from main to brprice/saturated-constructors-3 April 24, 2023 11:30
@brprice
Copy link
Copy Markdown
Contributor Author

brprice commented Apr 24, 2023

Same old question/todo:

Questions for reviewers: I'm not sure what conventional commit prefixes to use on a bunch of these, also I haven't worried about what changes are breaking -- for a dumb openapi frontend they should all be fine, but may break "smarter" clients, (and as with #958 something may well break the DB. I need to think about this).

Comment thread primer/src/Primer/Action/Available.hs Outdated
@brprice brprice force-pushed the brprice/saturated-constructors-3 branch from e905688 to ab4a10f Compare April 24, 2023 13:31
@brprice brprice force-pushed the brprice/saturated-constructors-4 branch from 4c2a561 to d26c774 Compare April 24, 2023 13:31
@brprice brprice force-pushed the brprice/saturated-constructors-3 branch from ab4a10f to da76901 Compare April 25, 2023 15:52
@brprice brprice force-pushed the brprice/saturated-constructors-4 branch 3 times, most recently from e49183d to edcec45 Compare April 26, 2023 14:57
@brprice brprice requested a review from a team April 26, 2023 15:37
@georgefst
Copy link
Copy Markdown
Contributor

Same old question/todo:

Questions for reviewers: I'm not sure what conventional commit prefixes to use on a bunch of these, also I haven't worried about what changes are breaking -- for a dumb openapi frontend they should all be fine, but may break "smarter" clients, (and as with #958 something may well break the DB. I need to think about this).

I have tended to use test: for commits which only affect tests, e.g. 32930e5. And I too haven't always being sure when to use refactor, possibly being too liberal by using it for commits which do alter the API of the Primer Haskell library, or change serialisation in the non-OpenAPI API.

Anyway, I think we agreed with @dhess recently that it's not worth worrying too much about CC prefixes on individual commits, with PR titles being more important (I had been doing the opposite, and not using prefixes on PR titles, but I will from now on).

@brprice brprice force-pushed the brprice/saturated-constructors-3 branch 3 times, most recently from 1d1ebdb to 981252c Compare May 10, 2023 16:20
@brprice brprice force-pushed the brprice/saturated-constructors-4 branch from edcec45 to a6e9d65 Compare May 10, 2023 16:27
brprice added 11 commits May 10, 2023 17:53
This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors. (Both versions of the
testsuite currently pass, but when constructors are checkable the old
version would fail.)
This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors. (Both versions of the
testsuite currently pass, but when constructors are checkable the old
version would fail.)
…only

This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors. (Both versions of the
testsuite currently pass, but when constructors are checkable the old
version would fail.)
This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors. (Both versions of the
testsuite currently pass, but when constructors are checkable the old
version would fail.)
This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors.
This ensures that they are consistent with constructors being
checkable only.

This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors.
…or/literal

This is a step towards only generating terms which are consistent with
treating constructors as checkable. That in itself is a step towards
being able to change the typechecker to treat constructors as
checkable, rather than synthesisable. This would then enable us to
drop indices from constructors.

Note that literals will be synthesisable, but constructors will only be checkable.

This commit slightly generalises what terms we generate, since we can
now generate primcons for non-hole types.
…e only

This is a step towards being able to change the typechecker to treat
constructors as checkable, rather than synthesisable. This would then
enable us to drop indices from constructors.
Add some tests which will show the difference in smartholes behaviour
once constructors are considered checkable. Currently they are
synthesisable, so smartholes puts a constructor directly inside a
hole.
Add some tests which will show the difference in action behaviour once
constructors are considered checkable. Currently they are
synthesisable, so they (via smartholes) put constructors directly
inside holes.
This has a knock-on effect on ConstructRefinedCon: if there is no
valid refinement (and we thus fall back to inserting a saturated
constructor in a hole), we need to put an annotation on the
constructor which is inside a hole.

BREAKING CHANGE: this will reject some programs that previous versions
of primer would consider valid. Thus it may break loading a database
that was generated by a older version of primer.
Now constructors are checkable, we statically know that case redexes
must be of the form `case (C @A... s... : T @A'...) of ...`, i.e. we
know that the constructor must be annotated. Thus we can remove some
logic dealing with the non-annotated case which was previously possible.
@brprice brprice force-pushed the brprice/saturated-constructors-4 branch from a6e9d65 to f00e897 Compare May 10, 2023 16:54
@brprice brprice force-pushed the brprice/saturated-constructors-3 branch from 981252c to 251c13d Compare May 10, 2023 16:54
@brprice brprice changed the title feat: saturated constructors stage 4 feat!: saturated constructors stage 4 May 11, 2023
Base automatically changed from brprice/saturated-constructors-3 to main May 11, 2023 08:52
@brprice brprice added this pull request to the merge queue May 11, 2023
Merged via the queue into main with commit 0443315 May 11, 2023
@brprice brprice deleted the brprice/saturated-constructors-4 branch May 11, 2023 09:02
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