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

Stdlib array extension #1195

Merged
merged 12 commits into from Mar 23, 2023
Merged

Stdlib array extension #1195

merged 12 commits into from Mar 23, 2023

Conversation

yannham
Copy link
Member

@yannham yannham commented Mar 23, 2023

Following #935, add a batch of missing functions to the array stdlib. This PR adds a slice primitive operation as well, required to implement the corresponding function of the stdlib. This PR adds a new enum stdlib module as well, moving a few functions/contracts from the string module.

Doing so, we also expanded the documentation of folds, by adding a more informal description as well as a paragraph on fold_right vs fold_left. This is useful as a reference for the documentation of reduce_right and reduce_left too.

Follow-up

We can now get rid of the %tail% primop, which is subsumed by %slice% 1 (%length% <.>). In the same spirit, we should probably get rid of %head% in favor of %elem_at% 0. This is planned in a follow-up PR.

@github-actions github-actions bot temporarily deployed to pull request March 23, 2023 11:51 Inactive
src/term/array.rs Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated Show resolved Hide resolved
stdlib/array.ncl Outdated
Comment on lines 561 to 566
if array == [] then
array
else
let first = %elem_at% array 0 in
let rest = %array_slice% 1 (%length% array) array in
fold_left f first rest,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think returning an empty array when given one makes sense here (How does this even typecheck?). It would probably be best to somehow fail on an empty array; maybe by applying the array.NonEmpty contract somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't make any sense, I think I thought about returning None and may brain then failed. We should fail on this, probably.

The issue I had with more specific contracts like NonEmpty is that they make those standard functions a pain to use in statically typed code. On one hand it's a shame, because we have those nice contracts with argument tracking and whatnot which could provide really good error messages. On the other hand, making folds, reduce and map a pain to use in typed code is also kind of a show stopper. One solution could be to separate them in submodules (having a more loosely typed version, and a tight contract-protected version), but that isn't satisfying either.

Maybe one solution at some point would be the ideas exposed in #420.

Copy link
Member

Choose a reason for hiding this comment

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

You could delegate the contract check explicitly to the runtime, by doing something like

reduce_left : forall a. (a -> a -> a) -> Array a -> Array a
  = fun f array' =>
    let array = array' | NonEmpty in
    ...

I wouldn't be great for error reporting, because the blame label would point into the standard library. But it would be usable in statically checked code now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would that work? Now array has type NonEmpty, and you can't do anything with it in typed code anymore, I fear.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe something like %seq% (array | NonEmpty) ...restofthefunction... but this starts to be really convoluted 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this calls for a construct to check a contract without influencing typechecking

Copy link
Member Author

Choose a reason for hiding this comment

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

How does this even typecheck?

Well, it doesn't, that's why the tests are failing 🙃

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, we could use a type annotation for the static type we want + an additional contract annotation, for more checks. Those won't influence static typechecking (which picks the first annotation it find as the apparent type). On one side it sounds slightly like a hack, but on the other, it's not too bad

Copy link
Member

Choose a reason for hiding this comment

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

So, we'd write

reduce_left
  : forall a. (a -> a -> a) -> Array a -> a
  | forall a. (a -> a -> a) -> NonEmpty -> a
  = ...

I guess? That's a little bit of duplication, but I think it's a price worth paying here 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

We can also just add the "differential" contract, so to speak:

reduce_left
  : forall a. (a -> a -> a) -> Array a -> a
  | Dyn -> NonEmpty -> Dyn

That might make for a slightly worse error message, though. We could also duplicate the type fully as you propose, and rely on contract elision (yet to be added) to not do too much work.

stdlib/enum.ncl Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request March 23, 2023 15:53 Inactive
@yannham yannham force-pushed the feature/stdlib-array-extension branch from 82db787 to 6592fd2 Compare March 23, 2023 17:04
@github-actions github-actions bot temporarily deployed to pull request March 23, 2023 17:08 Inactive
@yannham yannham merged commit e83f5c7 into master Mar 23, 2023
4 checks passed
@yannham yannham deleted the feature/stdlib-array-extension branch March 23, 2023 21:12
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.

None yet

2 participants