Conversation
tramhnt99
requested review from
anton-trunov,
jjcnn and
vaivaswatha
as code owners
…mpiler into uc_first_order
vaivaswatha
left a comment
vaivaswatha
left a comment
There was a problem hiding this comment.
Tram, thank you for this PR and apologies for the delayed review. I have a bunch of minor comments. Overall the approach seems fine to me, but I want to do another pass over this to be sure that we haven't missed out anything.
| (* The following analysis finds function candidates for Uncurrying. | ||
| Candidates must fulfill the following criteria to be uncurried: | ||
| * Have an arity of >= 2 | ||
| * Are never partiall applied |
| ~compare:(fun a a' -> Identifier.compare (fst a) (fst a')) | ||
| pl | ||
|
|
||
| (* Find is expressions is a Fun expr |
There was a problem hiding this comment.
Find is expressions if e is a Fun expr
| If a `Fun` expression's immediate subexpression is not `Fun`, | ||
| arity of the expression is 1. Otherwise, if the subexpression | ||
| if `Fun`, then the arity is one more then the immediate `Fun` | ||
| expression. |
| *) | ||
| let find_function_arity (e, annot) = | ||
| let rec iter_funcs (e', _) acc = | ||
| let res = |
There was a problem hiding this comment.
let res = ...
in
res
can be replaced by just the inner expression.
| (* if a pair (f,0) exists, f already won't be considered for uncurrying | ||
| (f,0) indicates that the function was used as a higher order function | ||
| *) | ||
| let is_to_uncur = |
There was a problem hiding this comment.
is_to_uncur can_uncurry
| (* Environment @ienv contains functions within scope that HAVE been uncurried | ||
| and their uncurried types *) | ||
| (* Function to uncurry Func expressions (wrapped in GasExpr) *) | ||
| let rec uncurry_func_epxr newname ienv fe = |
| "Uncurry: internal error: uncurry_func_epxr has no Gas rapped \ | ||
| around Fun." | ||
| in | ||
| let fun_rep' = |
There was a problem hiding this comment.
This is doing the same thing as let new_rep = ..., maybe you can have a small function that can be called at both places.
| * translate_expr does not return an environment because the scope of | ||
| newly defined functions in expr ends there | ||
| *) | ||
| (* If functions that are NOT uncurried, their App will be transformed apply arguments |
There was a problem hiding this comment.
Translate functions and their applications into the uncurried syntax. For functions that are uncurried (i.e., sequence of nested functions which are rewritten into a single function with multiple arguments), their applications will have multiple arguments too. For functions which aren't uncurried, they application will be transformed to apply single arguments in separate steps, because the AST must still follow uncurried semantics.
| ( UCS.GasExpr (StaticCost static_gas, (UCS.Fun (params', body'), fun_rep')), | ||
| new_rep' ) | ||
|
|
||
| (* Note: |
There was a problem hiding this comment.
This Note can go along with Note after the function description.
| | App (a, l) -> | ||
| let a' = translate_var a in | ||
| (* If the function was uncurried *) | ||
| if Identifier.is_mem_id a' (fst @@ List.unzip ienv) then |
There was a problem hiding this comment.
This involves two traversals over the list. It's simpler to just search for 'a in the zipped list by using List.mem directly
2 participants
Issue link: #24