-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add dedicated node and constructor for general custom contracts #1964
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yannham
force-pushed
the
task/contract-from-custom
branch
from
June 18, 2024 16:36
26eb13d
to
4aaef05
Compare
Use the previously introduced new primop in the stdlib, to migrate from naked functions to wrapped custom contracts everywhere in the stdlib.
Introduce the new contract constructor std.contract.custom in the manual, and make sure that this is now everywhere presented as the official way of building custom contracts. The section on custom contracts is also re-organized to make general custom contracts the last section, and present it as a last resort rather than the default way of building custom contracts. We also left some placeholder for to the soon-to-come validators.
yannham
force-pushed
the
task/contract-from-custom
branch
from
June 19, 2024 08:51
8a8b23a
to
522c330
Compare
In a previous commit, after introducing %contract/custom%, the stdlib was upgraded to use this new contract constructor. Doing so, in the contract.unstable module, many `std.contract.apply` were turned into `%contract/apply%` following the philosophy that the stdlib should avoid indirection through other stdlib functions bit rather directly use primops. However, it turns out `%contract/apply%` is not equivalent to `std.contract.apply`. The later also stack the current diagnostics, while the former is really only performing a function-like application, which does make a difference for error reporting - the version with `%contract/apply%` could erase existing error reporting data. This commit reverts those primops calls back to using `std.contract.apply`.
jneem
approved these changes
Jun 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continue to address #1466.
After introducing a special AST node and constructor for contracts built from a predicate in #1955, this PR introduces another variant for generic custom contracts. For backward compatibility reasons, we still need to support custom contracts given as naked functions, but this PR migrates examples, the stdlib and the documentation to using this new node through the
std.contract.custom
constructor. The only module that isn't migrated is theinternals
module, because it was harder to do without breaking things (it can surely be done, but it requires a bit more changes), and because those primitive contracts aren't really user-facing and thus it's less important for error messages and the like that they follow the new convention. As this PR is already sizeable, this is left for future work (or maybe we don't want to do it at all).The documentation is updated in way that already assumes the as-of-yet nonexistent
std.contract.from_validator
, planned in a follow-up PR, because it was harder to pretend that it won't exist, or put differently, it would have required more effort to update the documentation once again with this new intermediate type of custom contract than to just leave a placeholder and pretend it already exists.#1955 introduced predicates as functions, where the AST node also stores the function's parameter - the idea was to be more typed, and avoid having something else than a function lying in a
CustomContract
node. Unfortunately, I realized while working on this PR that custom contracts as predicates can actually also be match expressions. We could add a variant for things that are either functions or match expressions, but it starts to be verbose and to cause more code duplication. For now, I reverted to using a genericRichTerm
there, where the only hypothesis is that it's a WHNF that can be applied to some arguments.