-
Notifications
You must be signed in to change notification settings - Fork 204
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
feat: add syntax for specifying function type environments #2357
feat: add syntax for specifying function type environments #2357
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not confidant this approach with overridden_types
and manually trying to propagate modified types during monomorphization will be a good base to build upon for subsequent features, even if we want to replace it eventually.
It looks like we can only pass in closures directly to functions as arguments in this PR (e.g. passing them in a tuple does not work). Although the later does issue a type error instead of panicking.
I think exposing the environment type parameter to users would be less of a change and allow us to implement functions accepting closures, even nested within other types, in addition to functions returning closures in one go.
I understand, I'll add support for the syntax you mentioned in the issue - |
b1e9751
to
a94cdd2
Compare
I've made the following changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Let's add the environment generics change (mentioned in a comment below) as a separate PR. That should get us all the way towards releasing closures as a feature in Noir.
There's a minor style suggestion but it's not a blocker for this PR.
crates/nargo_cli/tests/execution_success/closure_explicit_types/src/main.nr
Show resolved
Hide resolved
* master: feat(ssa): Merge slices in if statements with witness conditions (#2347) chore: Separate frontend `Visibility` and `Distinctness` from the ABI (#2369) feat: add syntax for specifying function type environments (#2357) chore: Remove unused `Directive::Log` (#2366) chore: clippy fixes (#2365) chore: Extract bytecode from artifact files for backend integration test inputs (#2356) feat: Add trait definition representation in DefCollector and HIR (#2338) chore: Remove unused `Intrinsic::Println` (#2358) fix: Remove duplicte `T` in `expected T, found T` error on tuple assignment (#2360) chore(brillig): Fix brillig radix instruction return vector size (#2350) fix: Show types in error message in same order as in source code (#2353)
* master: (46 commits) chore: Remove `serde` from `noirc_frontend` (#2390) chore: allow parenthesizing in two type locations (#2388) chore(ci): automatically delete cache entries associated with closed PRs (#2342) feat: Perform more checks for compile-time arithmetic (#2380) chore: Remove `noirc_abi::FunctionSignature` and define in terms of HIR (#2372) feat: Update to `acvm` 0.22.0 (#2363) chore: Update committed ACIR artifacts (#2376) feat(ssa): Merge slices in if statements with witness conditions (#2347) chore: Separate frontend `Visibility` and `Distinctness` from the ABI (#2369) feat: add syntax for specifying function type environments (#2357) chore: Remove unused `Directive::Log` (#2366) chore: clippy fixes (#2365) chore: Extract bytecode from artifact files for backend integration test inputs (#2356) feat: Add trait definition representation in DefCollector and HIR (#2338) chore: Remove unused `Intrinsic::Println` (#2358) fix: Remove duplicte `T` in `expected T, found T` error on tuple assignment (#2360) chore(brillig): Fix brillig radix instruction return vector size (#2350) fix: Show types in error message in same order as in source code (#2353) chore: update noir-source-resolver from `1.1.2` to `^1.1.3` (#2349) chore(ci): Avoid writing to cache in workflows triggered by the merge queue (#2341) ...
* master: (34 commits) chore: Decouple `noirc_abi` from frontend by introducing `PrintableType` (#2373) feat(brillig): Added locations for brillig artifacts (#2415) feat: Report compilation warnings before errors (#2398) chore: Rework `CrateGraph` to only have one root crate (#2391) chore: clippy fix (#2408) chore(deps): bump rustls-webpki from 0.101.1 to 0.101.4 (#2404) fix(acir): Attach locations to MemoryOps in ACIR (#2389) feat: Use equivalence information from equality assertions to simplify circuit (#2378) chore: fix body expr span (#2402) feat(attributes): enable custom attributes (#2395) chore: Remove `serde` from `noirc_frontend` (#2390) chore: allow parenthesizing in two type locations (#2388) chore(ci): automatically delete cache entries associated with closed PRs (#2342) feat: Perform more checks for compile-time arithmetic (#2380) chore: Remove `noirc_abi::FunctionSignature` and define in terms of HIR (#2372) feat: Update to `acvm` 0.22.0 (#2363) chore: Update committed ACIR artifacts (#2376) feat(ssa): Merge slices in if statements with witness conditions (#2347) chore: Separate frontend `Visibility` and `Distinctness` from the ABI (#2369) feat: add syntax for specifying function type environments (#2357) ...
Description
Problem*
Work towards #2334
Summary*
In #2335 we allowed higher order with closure arguments to pass type-checking -- this PR implements the monomorphizer changes needed to make those calls work.
overridden_types
A new property
overridden_types
is added in the Monomorphizer. It allows us to change the type of an expression, for the currently monomorphized function instance only. It's used for adding capture environments to function types.propagate.rs
A function
propagate_overridden_types_expr
is added in a new file - propagate.rs. After we override the type of a parameter to add a capture group, we need to walk the body of the function and also update the types of all expressions where it is used.Documentation
This PR requires documentation updates when merged.
Additional Context
PR Checklist*
cargo fmt
on default settings.