From f035d405d819078f7e2d8f2081a1a2de8685bef8 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 28 Mar 2022 14:02:50 -0700 Subject: [PATCH] Support converting simple $(eval) expressions While supporting $(eval) in the general case is impossible, as it would require emitting code at runtime, it is possible to handle some special cases that are common throughout the code base. Specifically, an assignement expression (where the left hand side is constant) can be converted without needing to evaluate strings as code, as its whole right hand side is treated as one string. However, this eval with an assignemnt can only be used as a statement, not an expression. So it requires the eval to be either a top-level expression, or nested within other expressions that can be converted to statements such as $(foreach) or $(if). Bug: 226974242 Test: go test Change-Id: Ifc52ef9ab7d62a69251918fcde5463f80a98a542 --- mk2rbc/expr.go | 44 +++++-- mk2rbc/mk2rbc.go | 273 +++++++++++++++++++++++++++++++----------- mk2rbc/mk2rbc_test.go | 37 ++++++ mk2rbc/node.go | 25 ++++ 4 files changed, 301 insertions(+), 78 deletions(-) diff --git a/mk2rbc/expr.go b/mk2rbc/expr.go index dc16d1d2f..54bb6d124 100644 --- a/mk2rbc/expr.go +++ b/mk2rbc/expr.go @@ -221,11 +221,9 @@ func (xi *interpolateExpr) emitListVarCopy(gctx *generationContext) { } func (xi *interpolateExpr) transform(transformer func(expr starlarkExpr) starlarkExpr) starlarkExpr { - argsCopy := make([]starlarkExpr, len(xi.args)) - for i, arg := range xi.args { - argsCopy[i] = arg.transform(transformer) + for i := range xi.args { + xi.args[i] = xi.args[i].transform(transformer) } - xi.args = argsCopy if replacement := transformer(xi); replacement != nil { return replacement } else { @@ -591,11 +589,9 @@ func (cx *callExpr) transform(transformer func(expr starlarkExpr) starlarkExpr) if cx.object != nil { cx.object = cx.object.transform(transformer) } - argsCopy := make([]starlarkExpr, len(cx.args)) - for i, arg := range cx.args { - argsCopy[i] = arg.transform(transformer) + for i := range cx.args { + cx.args[i] = cx.args[i].transform(transformer) } - cx.args = argsCopy if replacement := transformer(cx); replacement != nil { return replacement } else { @@ -769,3 +765,35 @@ func isEmptyString(expr starlarkExpr) bool { x, ok := expr.(*stringLiteralExpr) return ok && x.literal == "" } + +func negateExpr(expr starlarkExpr) starlarkExpr { + switch typedExpr := expr.(type) { + case *notExpr: + return typedExpr.expr + case *inExpr: + typedExpr.isNot = !typedExpr.isNot + return typedExpr + case *eqExpr: + typedExpr.isEq = !typedExpr.isEq + return typedExpr + case *binaryOpExpr: + switch typedExpr.op { + case ">": + typedExpr.op = "<=" + return typedExpr + case "<": + typedExpr.op = ">=" + return typedExpr + case ">=": + typedExpr.op = "<" + return typedExpr + case "<=": + typedExpr.op = ">" + return typedExpr + default: + return ¬Expr{expr: expr} + } + default: + return ¬Expr{expr: expr} + } +} diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go index c881751d7..5f5288e73 100644 --- a/mk2rbc/mk2rbc.go +++ b/mk2rbc/mk2rbc.go @@ -86,7 +86,7 @@ var knownFunctions = map[string]interface { "filter": &simpleCallParser{name: baseName + ".filter", returnType: starlarkTypeList}, "filter-out": &simpleCallParser{name: baseName + ".filter_out", returnType: starlarkTypeList}, "firstword": &firstOrLastwordCallParser{isLastWord: false}, - "foreach": &foreachCallPaser{}, + "foreach": &foreachCallParser{}, "if": &ifCallParser{}, "info": &makeControlFuncParser{name: baseName + ".mkinfo"}, "is-board-platform": &simpleCallParser{name: baseName + ".board_platform_is", returnType: starlarkTypeBool, addGlobals: true}, @@ -117,6 +117,17 @@ var knownFunctions = map[string]interface { "wildcard": &simpleCallParser{name: baseName + ".expand_wildcard", returnType: starlarkTypeList}, } +// The same as knownFunctions, but returns a []starlarkNode instead of a starlarkExpr +var knownNodeFunctions = map[string]interface { + parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) []starlarkNode +}{ + "eval": &evalNodeParser{}, + "if": &ifCallNodeParser{}, + "inherit-product": &inheritProductCallParser{loadAlways: true}, + "inherit-product-if-exists": &inheritProductCallParser{loadAlways: false}, + "foreach": &foreachCallNodeParser{}, +} + // These are functions that we don't implement conversions for, but // we allow seeing their definitions in the product config files. var ignoredDefines = map[string]bool{ @@ -846,15 +857,19 @@ func (ctx *parseContext) findMatchingPaths(pattern []string) []string { return res } -func (ctx *parseContext) handleInheritModule(v mkparser.Node, args *mkparser.MakeString, loadAlways bool) []starlarkNode { +type inheritProductCallParser struct { + loadAlways bool +} + +func (p *inheritProductCallParser) parse(ctx *parseContext, v mkparser.Node, args *mkparser.MakeString) []starlarkNode { args.TrimLeftSpaces() args.TrimRightSpaces() pathExpr := ctx.parseMakeString(v, args) if _, ok := pathExpr.(*badExpr); ok { return []starlarkNode{ctx.newBadNode(v, "Unable to parse argument to inherit")} } - return ctx.handleSubConfig(v, pathExpr, loadAlways, func(im inheritedModule) starlarkNode { - return &inheritNode{im, loadAlways} + return ctx.handleSubConfig(v, pathExpr, p.loadAlways, func(im inheritedModule) starlarkNode { + return &inheritNode{im, p.loadAlways} }) } @@ -873,19 +888,12 @@ func (ctx *parseContext) handleVariable(v *mkparser.Variable) []starlarkNode { // $(error xxx) // $(call other-custom-functions,...) - // inherit-product(-if-exists) gets converted to a series of statements, - // not just a single expression like parseReference returns. So handle it - // separately at the beginning here. - if strings.HasPrefix(v.Name.Dump(), "call inherit-product,") { - args := v.Name.Clone() - args.ReplaceLiteral("call inherit-product,", "") - return ctx.handleInheritModule(v, args, true) - } - if strings.HasPrefix(v.Name.Dump(), "call inherit-product-if-exists,") { - args := v.Name.Clone() - args.ReplaceLiteral("call inherit-product-if-exists,", "") - return ctx.handleInheritModule(v, args, false) + if name, args, ok := ctx.maybeParseFunctionCall(v, v.Name); ok { + if kf, ok := knownNodeFunctions[name]; ok { + return kf.parse(ctx, v, args) + } } + return []starlarkNode{&exprNode{expr: ctx.parseReference(v, v.Name)}} } @@ -1030,49 +1038,19 @@ func (ctx *parseContext) parseCompare(cond *mkparser.Directive) starlarkExpr { otherOperand = xLeft } - not := func(expr starlarkExpr) starlarkExpr { - switch typedExpr := expr.(type) { - case *inExpr: - typedExpr.isNot = !typedExpr.isNot - return typedExpr - case *eqExpr: - typedExpr.isEq = !typedExpr.isEq - return typedExpr - case *binaryOpExpr: - switch typedExpr.op { - case ">": - typedExpr.op = "<=" - return typedExpr - case "<": - typedExpr.op = ">=" - return typedExpr - case ">=": - typedExpr.op = "<" - return typedExpr - case "<=": - typedExpr.op = ">" - return typedExpr - default: - return ¬Expr{expr: expr} - } - default: - return ¬Expr{expr: expr} - } - } - // If we've identified one of the operands as being a string literal, check // for some special cases we can do to simplify the resulting expression. if otherOperand != nil { if stringOperand == "" { if isEq { - return not(otherOperand) + return negateExpr(otherOperand) } else { return otherOperand } } if stringOperand == "true" && otherOperand.typ() == starlarkTypeBool { if !isEq { - return not(otherOperand) + return negateExpr(otherOperand) } else { return otherOperand } @@ -1228,6 +1206,37 @@ func (ctx *parseContext) parseCompareStripFuncResult(directive *mkparser.Directi right: xValue, isEq: !negate} } +func (ctx *parseContext) maybeParseFunctionCall(node mkparser.Node, ref *mkparser.MakeString) (name string, args *mkparser.MakeString, ok bool) { + ref.TrimLeftSpaces() + ref.TrimRightSpaces() + + words := ref.SplitN(" ", 2) + if !words[0].Const() { + return "", nil, false + } + + name = words[0].Dump() + args = mkparser.SimpleMakeString("", words[0].Pos()) + if len(words) >= 2 { + args = words[1] + } + args.TrimLeftSpaces() + if name == "call" { + words = args.SplitN(",", 2) + if words[0].Empty() || !words[0].Const() { + return "", nil, false + } + name = words[0].Dump() + if len(words) < 2 { + args = &mkparser.MakeString{} + } else { + args = words[1] + } + } + ok = true + return +} + // parses $(...), returning an expression func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeString) starlarkExpr { ref.TrimLeftSpaces() @@ -1242,7 +1251,7 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt // If it is a single word, it can be a simple variable // reference or a function call - if len(words) == 1 && !isMakeControlFunc(refDump) && refDump != "shell" { + if len(words) == 1 && !isMakeControlFunc(refDump) && refDump != "shell" && refDump != "eval" { if strings.HasPrefix(refDump, soongNsPrefix) { // TODO (asmundak): if we find many, maybe handle them. return ctx.newBadExpr(node, "SOONG_CONFIG_ variables cannot be referenced, use soong_config_get instead: %s", refDump) @@ -1281,28 +1290,14 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt return ctx.newBadExpr(node, "unknown variable %s", refDump) } - expr := &callExpr{name: words[0].Dump(), returnType: starlarkTypeUnknown} - args := mkparser.SimpleMakeString("", words[0].Pos()) - if len(words) >= 2 { - args = words[1] - } - args.TrimLeftSpaces() - if expr.name == "call" { - words = args.SplitN(",", 2) - if words[0].Empty() || !words[0].Const() { - return ctx.newBadExpr(node, "cannot handle %s", refDump) - } - expr.name = words[0].Dump() - if len(words) < 2 { - args = &mkparser.MakeString{} + if name, args, ok := ctx.maybeParseFunctionCall(node, ref); ok { + if kf, found := knownFunctions[name]; found { + return kf.parse(ctx, node, args) } else { - args = words[1] + return ctx.newBadExpr(node, "cannot handle invoking %s", name) } - } - if kf, found := knownFunctions[expr.name]; found { - return kf.parse(ctx, node, args) } else { - return ctx.newBadExpr(node, "cannot handle invoking %s", expr.name) + return ctx.newBadExpr(node, "cannot handle %s", refDump) } } @@ -1486,9 +1481,46 @@ func (p *ifCallParser) parse(ctx *parseContext, node mkparser.Node, args *mkpars } } -type foreachCallPaser struct{} +type ifCallNodeParser struct{} -func (p *foreachCallPaser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) starlarkExpr { +func (p *ifCallNodeParser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) []starlarkNode { + words := args.Split(",") + if len(words) != 2 && len(words) != 3 { + return []starlarkNode{ctx.newBadNode(node, "if function should have 2 or 3 arguments, found "+strconv.Itoa(len(words)))} + } + + ifn := &ifNode{expr: ctx.parseMakeString(node, words[0])} + cases := []*switchCase{ + { + gate: ifn, + nodes: ctx.parseNodeMakeString(node, words[1]), + }, + } + if len(words) == 3 { + cases = append(cases, &switchCase{ + gate: &elseNode{}, + nodes: ctx.parseNodeMakeString(node, words[2]), + }) + } + if len(cases) == 2 { + if len(cases[1].nodes) == 0 { + // Remove else branch if it has no contents + cases = cases[:1] + } else if len(cases[0].nodes) == 0 { + // If the if branch has no contents but the else does, + // move them to the if and negate its condition + ifn.expr = negateExpr(ifn.expr) + cases[0].nodes = cases[1].nodes + cases = cases[:1] + } + } + + return []starlarkNode{&switchNode{ssCases: cases}} +} + +type foreachCallParser struct{} + +func (p *foreachCallParser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) starlarkExpr { words := args.Split(",") if len(words) != 3 { return ctx.newBadExpr(node, "foreach function should have 3 arguments, found "+strconv.Itoa(len(words))) @@ -1520,6 +1552,71 @@ func (p *foreachCallPaser) parse(ctx *parseContext, node mkparser.Node, args *mk } } +func transformNode(node starlarkNode, transformer func(expr starlarkExpr) starlarkExpr) { + switch a := node.(type) { + case *ifNode: + a.expr = a.expr.transform(transformer) + case *switchCase: + transformNode(a.gate, transformer) + for _, n := range a.nodes { + transformNode(n, transformer) + } + case *switchNode: + for _, n := range a.ssCases { + transformNode(n, transformer) + } + case *exprNode: + a.expr = a.expr.transform(transformer) + case *assignmentNode: + a.value = a.value.transform(transformer) + case *foreachNode: + a.list = a.list.transform(transformer) + for _, n := range a.actions { + transformNode(n, transformer) + } + } +} + +type foreachCallNodeParser struct{} + +func (p *foreachCallNodeParser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) []starlarkNode { + words := args.Split(",") + if len(words) != 3 { + return []starlarkNode{ctx.newBadNode(node, "foreach function should have 3 arguments, found "+strconv.Itoa(len(words)))} + } + if !words[0].Const() || words[0].Empty() || !identifierFullMatchRegex.MatchString(words[0].Strings[0]) { + return []starlarkNode{ctx.newBadNode(node, "first argument to foreach function must be a simple string identifier")} + } + + loopVarName := words[0].Strings[0] + + list := ctx.parseMakeString(node, words[1]) + if list.typ() != starlarkTypeList { + list = &callExpr{ + name: baseName + ".words", + returnType: starlarkTypeList, + args: []starlarkExpr{list}, + } + } + + actions := ctx.parseNodeMakeString(node, words[2]) + // TODO(colefaust): Replace transforming code with something more elegant + for _, action := range actions { + transformNode(action, func(expr starlarkExpr) starlarkExpr { + if varRefExpr, ok := expr.(*variableRefExpr); ok && varRefExpr.ref.name() == loopVarName { + return &identifierExpr{loopVarName} + } + return nil + }) + } + + return []starlarkNode{&foreachNode{ + varName: loopVarName, + list: list, + actions: actions, + }} +} + type wordCallParser struct{} func (p *wordCallParser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) starlarkExpr { @@ -1630,6 +1727,31 @@ func (p *mathMaxOrMinCallParser) parse(ctx *parseContext, node mkparser.Node, ar } } +type evalNodeParser struct{} + +func (p *evalNodeParser) parse(ctx *parseContext, node mkparser.Node, args *mkparser.MakeString) []starlarkNode { + parser := mkparser.NewParser("Eval expression", strings.NewReader(args.Dump())) + nodes, errs := parser.Parse() + if errs != nil { + return []starlarkNode{ctx.newBadNode(node, "Unable to parse eval statement")} + } + + if len(nodes) == 0 { + return []starlarkNode{} + } else if len(nodes) == 1 { + switch n := nodes[0].(type) { + case *mkparser.Assignment: + if n.Name.Const() { + return ctx.handleAssignment(n) + } + case *mkparser.Comment: + return []starlarkNode{&commentNode{strings.TrimSpace("#" + n.Comment)}} + } + } + + return []starlarkNode{ctx.newBadNode(node, "Eval expression too complex; only assignments and comments are supported")} +} + func (ctx *parseContext) parseMakeString(node mkparser.Node, mk *mkparser.MakeString) starlarkExpr { if mk.Const() { return &stringLiteralExpr{mk.Dump()} @@ -1654,6 +1776,16 @@ func (ctx *parseContext) parseMakeString(node mkparser.Node, mk *mkparser.MakeSt return NewInterpolateExpr(parts) } +func (ctx *parseContext) parseNodeMakeString(node mkparser.Node, mk *mkparser.MakeString) []starlarkNode { + // Discard any constant values in the make string, as they would be top level + // string literals and do nothing. + result := make([]starlarkNode, 0, len(mk.Variables)) + for i := range mk.Variables { + result = append(result, ctx.handleVariable(&mk.Variables[i])...) + } + return result +} + // Handles the statements whose treatment is the same in all contexts: comment, // assignment, variable (which is a macro call in reality) and all constructs that // do not handle in any context ('define directive and any unrecognized stuff). @@ -1698,6 +1830,7 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) []starlarkNod if result == nil { result = []starlarkNode{} } + return result } diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go index 31739fa98..4fea778d4 100644 --- a/mk2rbc/mk2rbc_test.go +++ b/mk2rbc/mk2rbc_test.go @@ -1313,6 +1313,11 @@ BOOT_KERNEL_MODULES_FILTER_2 := $(foreach m,$(BOOT_KERNEL_MODULES_LIST),%/$(m)) FOREACH_WITH_IF := $(foreach module,\ $(BOOT_KERNEL_MODULES_LIST),\ $(if $(filter $(module),foo.ko),,$(error module "$(module)" has an error!))) + +# Same as above, but not assigning it to a variable allows it to be converted to statements +$(foreach module,\ + $(BOOT_KERNEL_MODULES_LIST),\ + $(if $(filter $(module),foo.ko),,$(error module "$(module)" has an error!))) `, expected: `load("//build/make/core:product_config.rbc", "rblf") @@ -1324,6 +1329,10 @@ def init(g, handle): g["BOOT_KERNEL_MODULES_LIST"] += ["bar.ko"] g["BOOT_KERNEL_MODULES_FILTER_2"] = ["%%/%s" % m for m in g["BOOT_KERNEL_MODULES_LIST"]] g["FOREACH_WITH_IF"] = [("" if rblf.filter(module, "foo.ko") else rblf.mkerror("product.mk", "module \"%s\" has an error!" % module)) for module in g["BOOT_KERNEL_MODULES_LIST"]] + # Same as above, but not assigning it to a variable allows it to be converted to statements + for module in g["BOOT_KERNEL_MODULES_LIST"]: + if not rblf.filter(module, "foo.ko"): + rblf.mkerror("product.mk", "module \"%s\" has an error!" % module) `, }, { @@ -1472,6 +1481,34 @@ LOCAL_PATH := $(call my-dir) def init(g, handle): cfg = rblf.cfg(handle) +`, + }, + { + desc: "Evals", + mkname: "product.mk", + in: ` +$(eval) +$(eval MY_VAR := foo) +$(eval # This is a test of eval functions) +$(eval $(TOO_COMPLICATED) := bar) +$(foreach x,$(MY_LIST_VAR), \ + $(eval PRODUCT_COPY_FILES += foo/bar/$(x):$(TARGET_COPY_OUT_VENDOR)/etc/$(x)) \ + $(if $(MY_OTHER_VAR),$(eval PRODUCT_COPY_FILES += $(MY_OTHER_VAR):foo/bar/$(x))) \ +) + +`, + expected: `load("//build/make/core:product_config.rbc", "rblf") + +def init(g, handle): + cfg = rblf.cfg(handle) + g["MY_VAR"] = "foo" + # This is a test of eval functions + rblf.mk2rbc_error("product.mk:5", "Eval expression too complex; only assignments and comments are supported") + for x in rblf.words(g.get("MY_LIST_VAR", "")): + rblf.setdefault(handle, "PRODUCT_COPY_FILES") + cfg["PRODUCT_COPY_FILES"] += ("foo/bar/%s:%s/etc/%s" % (x, g.get("TARGET_COPY_OUT_VENDOR", ""), x)).split() + if g.get("MY_OTHER_VAR", ""): + cfg["PRODUCT_COPY_FILES"] += ("%s:foo/bar/%s" % (g.get("MY_OTHER_VAR", ""), x)).split() `, }, } diff --git a/mk2rbc/node.go b/mk2rbc/node.go index 9d5af91c4..c0c4c98e8 100644 --- a/mk2rbc/node.go +++ b/mk2rbc/node.go @@ -294,3 +294,28 @@ func (ssw *switchNode) emit(gctx *generationContext) { ssCase.emit(gctx) } } + +type foreachNode struct { + varName string + list starlarkExpr + actions []starlarkNode +} + +func (f *foreachNode) emit(gctx *generationContext) { + gctx.newLine() + gctx.writef("for %s in ", f.varName) + f.list.emit(gctx) + gctx.write(":") + gctx.indentLevel++ + hasStatements := false + for _, a := range f.actions { + if _, ok := a.(*commentNode); !ok { + hasStatements = true + } + a.emit(gctx) + } + if !hasStatements { + gctx.emitPass() + } + gctx.indentLevel-- +}