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

Closed contracts #374

Merged
merged 8 commits into from Sep 1, 2021
Merged

Closed contracts #374

merged 8 commits into from Sep 1, 2021

Conversation

yannham
Copy link
Member

@yannham yannham commented Aug 5, 2021

Depend on #370. Fix #303.

Summary

This PR implements the fix proposed in #303: it makes the contract application operator let Contract = { a | Num } in value | #Contract more restrictive than just value & Contract, in that it now raises a contract violation if value has an extra field other than a.

Contract application having a different behavior than merge for record contracts is nice, because beside contract application now doing the correct thing (by default a schema shouldn't allow extra fields), one can still use standard merging on closed contracts to compose them:

let ContractA = {a | Num} in
let ContractB = {b | Num} in
let Contract= ContractA & ContractB in
{a = 1, b = 2} | #Contract

Open contracts

This PR introduces a new ellipsis syntax .. to stipulate that Contract allows extra field, as in the working let Contract = {a | Num, ..} in {a = 1, b = 2} | #Contract . With the ellipsis, a record contract behaves as before this PR.

The behavior with respect to merging is that if any of the operands is open, then the resulting contract is open. The rationale is that .. can be imagined as a special field that is syntactically merged as any other field: {a | Num} & {..} = {a | Num, ..}.

@yannham yannham changed the base branch from master to task/grammar-use-precedence August 5, 2021 09:11
@github-actions github-actions bot temporarily deployed to pull request August 5, 2021 09:14 Inactive
@yannham yannham marked this pull request as ready for review August 5, 2021 11:33
@github-actions github-actions bot temporarily deployed to pull request August 5, 2021 11:37 Inactive
@yannham yannham linked an issue Aug 5, 2021 that may be closed by this pull request
@github-actions github-actions bot temporarily deployed to pull request August 6, 2021 13:39 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 10, 2021 18:39 Inactive
@yannham yannham force-pushed the task/grammar-use-precedence branch from 7c2fe49 to 7199f18 Compare August 31, 2021 15:06
@github-actions github-actions bot temporarily deployed to pull request August 31, 2021 15:22 Inactive
Base automatically changed from task/grammar-use-precedence to master August 31, 2021 16:54
@dpulls
Copy link

dpulls bot commented Aug 31, 2021

🎉 All dependencies have been resolved !

@yannham yannham added area: contracts merge-queue merge on green CI labels Sep 1, 2021
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

I really like this.

Just a very minor comment that you can feel free to ignore, but otherwise 👍

Comment on lines +154 to +166
impl Default for RecordAttrs {
fn default() -> Self {
RecordAttrs { open: false }
}
}

impl RecordAttrs {
pub fn merge(attrs1: RecordAttrs, attrs2: RecordAttrs) -> RecordAttrs {
RecordAttrs {
open: attrs1.open || attrs2.open,
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

(nit) This looks a lot like a monoid. Isn’t there a standard trait for that in Rust?

Copy link
Member Author

@yannham yannham Sep 1, 2021

Choose a reason for hiding this comment

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

Good point (and typical Haskeller 😛 ), but I can't find one in the standard library, just in some third-party crates, so I guess I'll leave it as it is.

@github-actions github-actions bot temporarily deployed to pull request September 1, 2021 10:32 Inactive
@yannham yannham merged commit da4de0b into master Sep 1, 2021
@yannham yannham deleted the task/record-contracts-repr branch September 1, 2021 10:40
yannham added a commit that referenced this pull request Sep 2, 2021
Closed contracts

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

Successfully merging this pull request may close these issues.

Closed record contracts
2 participants