Skip to content

KC: Integrate Common Validated Proto Types #26

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jan 30, 2023
Merged

KC: Integrate Common Validated Proto Types #26

merged 11 commits into from
Jan 30, 2023

Conversation

cstml
Copy link
Contributor

@cstml cstml commented Jan 25, 2023 •
edited
Loading

  • swap the existing interim types for the proto validated ones in the compiler
  • improve data structures - i.e. use Map instead of lists, Text etc.
  • break up the inference into smaller modules.
  • add tests.
  • add some missing low hanging class instances.

@cstml cstml force-pushed the compiler/common-types-kind-checker branch from 3126f50 to eca2810 Compare January 25, 2023 13:00
@cstml cstml marked this pull request as draft January 25, 2023 13:00
@cstml cstml force-pushed the compiler/common-types-kind-checker branch from eca2810 to a51fe83 Compare January 25, 2023 20:18
gnumonik added a commit that referenced this pull request Jan 26, 2023
@gnumonik
merged #26
5cc0c59
@cstml cstml force-pushed the compiler/common-types-kind-checker branch from a51fe83 to 9a02484 Compare January 26, 2023 15:55
Base automatically changed from common/proto-types-validated to main January 26, 2023 18:25
@cstml cstml requested review from bladyjoker and gnumonik January 30, 2023 11:30
@cstml cstml added the compiler Work related to the Lambda Buffers Compiler label Jan 30, 2023
@cstml cstml added this to the Catalyst milestone 2 milestone Jan 30, 2023
@cstml cstml self-assigned this Jan 30, 2023
@cstml cstml marked this pull request as ready for review January 30, 2023 11:31
bladyjoker
bladyjoker approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@bladyjoker bladyjoker left a comment

Choose a reason for hiding this comment

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

Good stuff, please take a look at and integrate KC into the CLI

lambda-buffers/lambda-buffers-compiler/app/LambdaBuffers/Compiler/Cli/Compile.hs

Line 27 in 3895701

case fromProto @CompilerInput @ProtoCompat.CompilerInput compIn of

lambda-buffers-compiler/src/LambdaBuffers/Compiler/KindCheck.hs
Comment on lines 49 to 50
data KindCheckFailure
= CheckFailure String
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's expose that in the API. Try to think of what errors make sense to expose there and in what granularity.

lambda-buffers-compiler/src/LambdaBuffers/Compiler/KindCheck.hs
Comment on lines +191 to +193
-- X To Canonical type conversion functions.

-- | TyDef to Kind Canonical representation.
Copy link
Contributor

Choose a reason for hiding this comment

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

When you have the cycles, having something like https://github.com/mlabs-haskell/lambda-buffers/blob/5d74a0cc1bc1046494b244440d33c52cf4fefa50/lambda-buffers-compiler/src/LambdaBuffers/Compiler/Canonical.hs would be good

@bladyjoker bladyjoker merged commit d7291b3 into main Jan 30, 2023
@bladyjoker bladyjoker deleted the compiler/common-types-kind-checker branch January 30, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnumonik gnumonik Awaiting requested review from gnumonik

1 more reviewer

@bladyjoker bladyjoker bladyjoker approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@cstml cstml

Labels
compiler Work related to the Lambda Buffers Compiler
Projects
Lambda Buffers
Status: Done
Milestone
Catalyst milestone 2: End to end proo...
Development

Successfully merging this pull request may close these issues.
2 participants
@cstml @bladyjoker