Skip to content

Commit

Permalink
Keep object locals only once in AST (#263)
Browse files Browse the repository at this point in the history
Keep object locals only once in AST

For example this reduces the size of stdlib ast file
roughly 3x. Note that this change doesn't regenerate the stdlib,
so that the diff here is sane.

It is likely to slightly improve performance of code using
a lot of locals (~10% on bench.05.gen.jsonnet).

The desugaring is more strightforward now, and we're back
to desugaring each node exactly once.
  • Loading branch information
sbarzowski authored May 7, 2019
1 parent 181c86d commit 33b6dcf
Show file tree
Hide file tree
Showing 18 changed files with 280 additions and 171 deletions.
1 change: 1 addition & 0 deletions ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ type DesugaredObject struct {
NodeBase
Asserts Nodes
Fields DesugaredObjectFields
Locals LocalBinds
}

// ---------------------------------------------------------------------------
Expand Down
24 changes: 23 additions & 1 deletion builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,9 @@ func builtinStrReplace(i *interpreter, trace TraceElement, strv, fromv, tov valu
}

func builtinUglyObjectFlatMerge(i *interpreter, trace TraceElement, x value) (value, error) {
// TODO(sbarzowski) consider keeping comprehensions in AST
// It will probably be way less hacky, with better error messages and better performance

objarr, err := i.getArray(x, trace)
if err != nil {
return nil, err
Expand All @@ -818,30 +821,49 @@ func builtinUglyObjectFlatMerge(i *interpreter, trace TraceElement, x value) (va
return &valueSimpleObject{}, nil
}
newFields := make(simpleObjectFieldMap)
var locals []objectLocal
var upValues bindingFrame
for _, elem := range objarr.elements {
obj, err := i.evaluateObject(elem, trace)
if err != nil {
return nil, err
}
// starts getting ugly - we mess with object internals
simpleObj := obj.(*valueSimpleObject)
// there is only one field, really
for fieldName, fieldVal := range simpleObj.fields {
if _, alreadyExists := newFields[fieldName]; alreadyExists {
return nil, i.Error(duplicateFieldNameErrMsg(fieldName), trace)
}

// Here is the tricky part. Each field in a comprehension has different
// upValues, because for example in {[v]: v for v in ["x", "y", "z"] },
// the v is different for each field.
// Yet, even though upValues are field-specific, they are shadowed by object locals,
// so we need to make holes to let them pass through
upValues := simpleObj.upValues
for _, l := range simpleObj.locals {
delete(upValues, l.name)
}

newFields[fieldName] = simpleObjectField{
hide: fieldVal.hide,
field: &bindingsUnboundField{
inner: fieldVal.field,
bindings: simpleObj.upValues,
},
}
// another ugliness - we just take the locals of our last object,
// we assume that the locals are the same for each of merged objects
locals = simpleObj.locals
}
}

return makeValueSimpleObject(
nil, // no binding frame
upValues,
newFields,
[]unboundField{}, // No asserts allowed
locals,
), nil
}

Expand Down
Loading

0 comments on commit 33b6dcf

Please sign in to comment.