-
Notifications
You must be signed in to change notification settings - Fork 71
integrate resolver's lambda unraveling into semantic translator pass #6295
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
Conversation
This commit integrates the lambda unraveling technique used in the resolver into the first-pass of the semantic translator. This is possible by indexing the lambda permutation using decl IDs instead of call tags based on the intuition that the only thing that can give rise to variation in unraveled combinations is the declared function, i.e., at the outermost call site, each variation of lamba-parameterized function depends only on the function passed in, which are uniquely defined by their ID not their unraveled instances. This means the evaluator can simply traverse an expression to determine if it's constant and then dagen it. There is no need anymore to re-enter the resolver. This is also the key step needed to integrate type checking into the translator, which will then allow us to utilize dataflow-analyzed fused types from pipe queries in schema-column resolution for SQL queries that include pipe subqueries. This will be forthcoming in a subsequent PR.
nwt
left a comment
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 still working on understanding the nuance in here but it looks good at a high level and it clearly works so
.
compiler/semantic/expr.go
Outdated
| id := e.Name | ||
| if boundID, _ := t.scope.lookupFuncDeclOrParam(e.Name); boundID != "" { |
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.
Nit: Using ID here would make the data flow a little clearer.
| id := e.Name | |
| if boundID, _ := t.scope.lookupFuncDeclOrParam(e.Name); boundID != "" { | |
| id := e.Name | |
| if boundID, _ := t.scope.lookupFuncDeclOrParam(id); boundID != "" { |
compiler/semantic/resolver.go
Outdated
| // at a built-in // or function declaration and they can't be modified | ||
| // *(only passed by reference), the variants are determined by decl ID |
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.
Nit:
| // at a built-in // or function declaration and they can't be modified | |
| // *(only passed by reference), the variants are determined by decl ID | |
| // at a built-in or function declaration and they can't be modified | |
| // (only passed by reference), the variants are determined by decl ID |
compiler/semantic/resolver.go
Outdated
| } | ||
|
|
||
| type lambda struct { | ||
| param string // parameter name this lambda arg appeared asa |
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.
Nit:
| param string // parameter name this lambda arg appeared asa | |
| param string // parameter name this lambda arg appeared as |
compiler/semantic/resolver.go
Outdated
| params := idsToStrings(d.lambda.Params) | ||
| for _, param := range idsToStrings(d.lambda.Params) { |
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.
Nit:
| params := idsToStrings(d.lambda.Params) | |
| for _, param := range idsToStrings(d.lambda.Params) { | |
| params := idsToStrings(d.lambda.Params) | |
| for _, param := range params { |
compiler/semantic/expr.go
Outdated
| func (t *translator) resolveCall(n ast.Node, id string, args []sem.Expr) sem.Expr { | ||
| if isBuiltin(id) { | ||
| // Check argument count here for builtin functions. | ||
| if _, err := function.New(super.NewContext(), id, len(args)); err != nil { | ||
| t.error(n, err) | ||
| return badExpr() | ||
| } | ||
| return sem.NewCall(n, id, args) | ||
| } | ||
| return t.resolver.mustResolveCall(n, id, args) | ||
| } | ||
|
|
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.
Nit: Putting this immediately before mustResolveCall would improve readability a bit.
This commit integrates the lambda unraveling technique used in the resolver into the first-pass of the semantic translator. This is possible by indexing the lambda permutation using decl IDs instead of call tags based on the intuition that the only thing that can give rise to variation in unraveled combinations is the declared function, i.e., at the outermost call site, each variation of lamba-parameterized function depends only on the function passed in, which are uniquely defined by their ID not their unraveled instances.
This means the evaluator can simply traverse an expression to determine if it's constant and then dagen it. There is no need anymore to re-enter the resolver.
This is also the key step needed to integrate type checking into the translator, which will then allow us to utilize dataflow-analyzed fused types from pipe queries in schema-column resolution for SQL queries that include pipe subqueries. This will be forthcoming in a subsequent PR.