fix: Try to turn acir lambdas into brillig#7279
Closed
Conversation
TomAFrench
reviewed
Feb 4, 2025
Comment on lines
+505
to
+507
| self.in_unconstrained_function = true; | ||
| let res = self.block(block.statements); | ||
| self.in_unconstrained_function = was_in_unconstrained_function; |
Member
There was a problem hiding this comment.
This seems like we're treating the contents of an unsafe block as unconstrained. This isn't the case however as it's still constrained but the only difference is that we can call into unconstrained functions.
aakoshh
commented
Feb 5, 2025
Comment on lines
-239
to
+242
| let target_fns = | ||
| signature_to_functions_as_value.get(&dispatch_signature).cloned().unwrap_or_default(); | ||
| variants.insert((dispatch_signature, caller_runtime), target_fns); | ||
| let key = (dispatch_signature, caller_runtime); | ||
| let target_fns = signature_to_functions_as_value.get(&key).cloned().unwrap_or_default(); |
Contributor
Author
There was a problem hiding this comment.
With this change we end up in a situation where if the lambda is created in constrained code, and subsequently is passed to unconstrained code, then this list is empty and the compiler crashes further down.
5 tasks
Contributor
This file contains hidden or 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
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.
Description
Problem*
Resolves #7247
Summary*
Draft because it's unclear how to fix the
unconstrainedflag assigned to lambdas by theMonomorphizer.The lambda expression itself inherits the flag from where it's defined, rather than how it's going to be used. I changed it so that if it's created in an
unsafeblock then it becomesunconstrained(which I now understand is not correct), but there are examples such asarray::sorttest where we create a lambda in a constrained function, pass it to another constrained function, which uses it in its body in anunsafeblock to pass it on to anunconstrainedfunction it calls. It could call it itself outside theunsafeblock as well, which means afunctionparameter would potentially need to be bothbrilligandacir, depending on which block it's called from. Yet, we pass a single function ID in SSA, so how could it be both?I thought about changing it to
unconstrained ordering: fn[Env](T, T) -> bool, and then when we callsort_viawe could look at its call parameter types to divine that the lambda should be unconstrained, but if we maintain that it can be called in two ways, then I'm not sure how to do it, because the lambda argument is turned into a function before we process the function we pass it to, so the information of how it's used might not be available until later. Also, the user doesn't have to add this at the moment, in which case anacirlambda is created, and in the SSA we are stuck with the situation we wished to avoid, withbrilligcallingacir.I also considered flipping the flag in the lambda function when we process the function body of its caller, but there is no 1:1 correspondence between the callee and the lambda, e.g. multiple lambdas can be passed to it, so we would have to somehow remember which lambdas were passed to which functions.
Perhaps if we process the callee function first, we can gather enough information about how the arguments are used, but it sounds like it would flip everything on its head to process dependencies first, rather than queue them for later. Maybe the SSA pass like the runtime separation used to be is the more convenient way to clone lambdas with caller-specific runtimes.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.