Allow generic if statements

Previously, only comparisons between strings/variable references
and other strings/variable references or certain function calls
were allowed. After this cl, any two starlark expressions can
be compared. If they have different types, they will be converted
to strings first.

Fixes: 205995738
Bug: 201700692
Test: go test
Change-Id: Ib463de7b24d3abcd032dbdcba7c3f1351c314d01

mk2rbc/expr.go

@@ -197,6 +197,51 @@ func (v *variableRefExpr) emitListVarCopy(gctx *generationContext) {
 	}
 }
 
+type toStringExpr struct {
+	expr starlarkExpr
+}
+
+func (s *toStringExpr) eval(valueMap map[string]starlarkExpr) (res starlarkExpr, same bool) {
+	if x, same := s.expr.eval(valueMap); same {
+		res = s
+	} else {
+		res = &toStringExpr{expr: x}
+	}
+	return
+}
+
+func (s *toStringExpr) emit(ctx *generationContext) {
+	switch s.expr.typ() {
+	case starlarkTypeString, starlarkTypeUnknown:
+		// Assume unknown types are strings already.
+		s.expr.emit(ctx)
+	case starlarkTypeList:
+		ctx.write(`" ".join(`)
+		s.expr.emit(ctx)
+		ctx.write(")")
+	case starlarkTypeInt:
+		ctx.write(`("%d" % (`)
+		s.expr.emit(ctx)
+		ctx.write("))")
+	case starlarkTypeBool:
+		ctx.write("((")
+		s.expr.emit(ctx)
+		ctx.write(`) ? "true" : "")`)
+	case starlarkTypeVoid:
+		ctx.write(`""`)
+	default:
+		panic("Unknown starlark type!")
+	}
+}
+
+func (s *toStringExpr) typ() starlarkType {
+	return starlarkTypeString
+}
+
+func (s *toStringExpr) emitListVarCopy(gctx *generationContext) {
+	s.emit(gctx)
+}
+
 type notExpr struct {
 	expr starlarkExpr
 }
@@ -255,6 +300,12 @@ func (eq *eqExpr) emit(gctx *generationContext) {
 		return
 
 	}
+
+	if eq.left.typ() != eq.right.typ() {
+		eq.left = &toStringExpr{expr: eq.left}
+		eq.right = &toStringExpr{expr: eq.right}
+	}
+
 	// General case
 	eq.left.emit(gctx)
 	if eq.isEq {

mk2rbc/mk2rbc.go

@@ -1044,48 +1044,54 @@ func (ctx *parseContext) parseCompare(cond *mkparser.Directive) starlarkExpr {
 	args[1].TrimLeftSpaces()
 
 	isEq := !strings.HasSuffix(cond.Name, "neq")
-	switch xLeft := ctx.parseMakeString(cond, args[0]).(type) {
+	xLeft := ctx.parseMakeString(cond, args[0])
-	case *stringLiteralExpr, *variableRefExpr:
+	xRight := ctx.parseMakeString(cond, args[1])
-		switch xRight := ctx.parseMakeString(cond, args[1]).(type) {
+	if bad, ok := xLeft.(*badExpr); ok {
-		case *stringLiteralExpr, *variableRefExpr:
+		return bad
-			return &eqExpr{left: xLeft, right: xRight, isEq: isEq}
-		case *badExpr:
-			return xRight
-		default:
-			expr, ok := ctx.parseCheckFunctionCallResult(cond, xLeft, args[1])
-			if ok {
-				return expr
-			}
-			return ctx.newBadExpr(cond, "right operand is too complex: %s", args[1].Dump())
-		}
-	case *badExpr:
-		return xLeft
-	default:
-		switch xRight := ctx.parseMakeString(cond, args[1]).(type) {
-		case *stringLiteralExpr, *variableRefExpr:
-			expr, ok := ctx.parseCheckFunctionCallResult(cond, xRight, args[0])
-			if ok {
-				return expr
-			}
-			return ctx.newBadExpr(cond, "left operand is too complex: %s", args[0].Dump())
-		case *badExpr:
-			return xRight
-		default:
-			return ctx.newBadExpr(cond, "operands are too complex: (%s,%s)", args[0].Dump(), args[1].Dump())
-		}
 	}
+	if bad, ok := xRight.(*badExpr); ok {
+		return bad
+	}
+
+	if expr, ok := ctx.parseCompareSpecialCases(cond, xLeft, xRight); ok {
+		return expr
+	}
+
+	return &eqExpr{left: xLeft, right: xRight, isEq: isEq}
 }
 
-func (ctx *parseContext) parseCheckFunctionCallResult(directive *mkparser.Directive, xValue starlarkExpr,
+// Given an if statement's directive and the left/right starlarkExprs,
-	varArg *mkparser.MakeString) (starlarkExpr, bool) {
+// check if the starlarkExprs are one of a few hardcoded special cases
-	mkSingleVar, ok := varArg.SingleVariable()
+// that can be converted to a simpler equalify expression than simply comparing
-	if !ok {
+// the two.
+func (ctx *parseContext) parseCompareSpecialCases(directive *mkparser.Directive, left starlarkExpr,
+	right starlarkExpr) (starlarkExpr, bool) {
+	isEq := !strings.HasSuffix(directive.Name, "neq")
+
+	// All the special cases require a call on one side and a
+	// string literal/variable on the other. Turn the left/right variables into
+	// call/value variables, and return false if that's not possible.
+	var value starlarkExpr = nil
+	call, ok := left.(*callExpr)
+	if ok {
+		switch right.(type) {
+		case *stringLiteralExpr, *variableRefExpr:
+			value = right
+		}
+	} else {
+		call, _ = right.(*callExpr)
+		switch left.(type) {
+		case *stringLiteralExpr, *variableRefExpr:
+			value = left
+		}
+	}
+
+	if call == nil || value == nil {
 		return nil, false
 	}
-	expr := ctx.parseReference(directive, mkSingleVar)
+
-	negate := strings.HasSuffix(directive.Name, "neq")
 	checkIsSomethingFunction := func(xCall *callExpr) starlarkExpr {
-		s, ok := maybeString(xValue)
+		s, ok := maybeString(value)
 		if !ok || s != "true" {
 			return ctx.newBadExpr(directive,
 				fmt.Sprintf("the result of %s can be compared only to 'true'", xCall.name))
@@ -1095,97 +1101,90 @@ func (ctx *parseContext) parseCheckFunctionCallResult(directive *mkparser.Direct
 		}
 		return nil
 	}
-	switch x := expr.(type) {
-	case *callExpr:
-		switch x.name {
-		case "filter":
-			return ctx.parseCompareFilterFuncResult(directive, x, xValue, !negate), true
-		case "filter-out":
-			return ctx.parseCompareFilterFuncResult(directive, x, xValue, negate), true
-		case "wildcard":
-			return ctx.parseCompareWildcardFuncResult(directive, x, xValue, negate), true
-		case "findstring":
-			return ctx.parseCheckFindstringFuncResult(directive, x, xValue, negate), true
-		case "strip":
-			return ctx.parseCompareStripFuncResult(directive, x, xValue, negate), true
-		case "is-board-platform":
-			if xBad := checkIsSomethingFunction(x); xBad != nil {
-				return xBad, true
-			}
-			return &eqExpr{
-				left:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
-				right: x.args[0],
-				isEq:  !negate,
-			}, true
-		case "is-board-platform-in-list":
-			if xBad := checkIsSomethingFunction(x); xBad != nil {
-				return xBad, true
-			}
-			return &inExpr{
-				expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
-				list:  maybeConvertToStringList(x.args[0]),
-				isNot: negate,
-			}, true
-		case "is-product-in-list":
-			if xBad := checkIsSomethingFunction(x); xBad != nil {
-				return xBad, true
-			}
-			return &inExpr{
-				expr:  &variableRefExpr{ctx.addVariable("TARGET_PRODUCT"), true},
-				list:  maybeConvertToStringList(x.args[0]),
-				isNot: negate,
-			}, true
-		case "is-vendor-board-platform":
-			if xBad := checkIsSomethingFunction(x); xBad != nil {
-				return xBad, true
-			}
-			s, ok := maybeString(x.args[0])
-			if !ok {
-				return ctx.newBadExpr(directive, "cannot handle non-constant argument to is-vendor-board-platform"), true
-			}
-			return &inExpr{
-				expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
-				list:  &variableRefExpr{ctx.addVariable(s + "_BOARD_PLATFORMS"), true},
-				isNot: negate,
-			}, true
 
-		case "is-board-platform2", "is-board-platform-in-list2":
+	switch call.name {
-			if s, ok := maybeString(xValue); !ok || s != "" {
+	case "filter":
-				return ctx.newBadExpr(directive,
+		return ctx.parseCompareFilterFuncResult(directive, call, value, isEq), true
-					fmt.Sprintf("the result of %s can be compared only to empty", x.name)), true
+	case "filter-out":
-			}
+		return ctx.parseCompareFilterFuncResult(directive, call, value, !isEq), true
-			if len(x.args) != 1 {
+	case "wildcard":
-				return ctx.newBadExpr(directive, "%s requires an argument", x.name), true
+		return ctx.parseCompareWildcardFuncResult(directive, call, value, !isEq), true
-			}
+	case "findstring":
-			cc := &callExpr{
+		return ctx.parseCheckFindstringFuncResult(directive, call, value, !isEq), true
-				name:       x.name,
+	case "strip":
-				args:       []starlarkExpr{x.args[0]},
+		return ctx.parseCompareStripFuncResult(directive, call, value, !isEq), true
-				returnType: starlarkTypeBool,
+	case "is-board-platform":
-			}
+		if xBad := checkIsSomethingFunction(call); xBad != nil {
-			if !negate {
+			return xBad, true
-				return &notExpr{cc}, true
-			}
-			return cc, true
-		case "is-vendor-board-qcom":
-			if s, ok := maybeString(xValue); !ok || s != "" {
-				return ctx.newBadExpr(directive,
-					fmt.Sprintf("the result of %s can be compared only to empty", x.name)), true
-			}
-			// if the expression is ifneq (,$(call is-vendor-board-platform,...)), negate==true,
-			// so we should set inExpr.isNot to false
-			return &inExpr{
-				expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
-				list:  &variableRefExpr{ctx.addVariable("QCOM_BOARD_PLATFORMS"), true},
-				isNot: !negate,
-			}, true
-		default:
-			return ctx.newBadExpr(directive, "Unknown function in ifeq: %s", x.name), true
 		}
-	case *badExpr:
+		return &eqExpr{
-		return x, true
+			left:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
-	default:
+			right: call.args[0],
-		return nil, false
+			isEq:  isEq,
+		}, true
+	case "is-board-platform-in-list":
+		if xBad := checkIsSomethingFunction(call); xBad != nil {
+			return xBad, true
+		}
+		return &inExpr{
+			expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+			list:  maybeConvertToStringList(call.args[0]),
+			isNot: !isEq,
+		}, true
+	case "is-product-in-list":
+		if xBad := checkIsSomethingFunction(call); xBad != nil {
+			return xBad, true
+		}
+		return &inExpr{
+			expr:  &variableRefExpr{ctx.addVariable("TARGET_PRODUCT"), true},
+			list:  maybeConvertToStringList(call.args[0]),
+			isNot: !isEq,
+		}, true
+	case "is-vendor-board-platform":
+		if xBad := checkIsSomethingFunction(call); xBad != nil {
+			return xBad, true
+		}
+		s, ok := maybeString(call.args[0])
+		if !ok {
+			return ctx.newBadExpr(directive, "cannot handle non-constant argument to is-vendor-board-platform"), true
+		}
+		return &inExpr{
+			expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+			list:  &variableRefExpr{ctx.addVariable(s + "_BOARD_PLATFORMS"), true},
+			isNot: !isEq,
+		}, true
+
+	case "is-board-platform2", "is-board-platform-in-list2":
+		if s, ok := maybeString(value); !ok || s != "" {
+			return ctx.newBadExpr(directive,
+				fmt.Sprintf("the result of %s can be compared only to empty", call.name)), true
+		}
+		if len(call.args) != 1 {
+			return ctx.newBadExpr(directive, "%s requires an argument", call.name), true
+		}
+		cc := &callExpr{
+			name:       call.name,
+			args:       []starlarkExpr{call.args[0]},
+			returnType: starlarkTypeBool,
+		}
+		if isEq {
+			return &notExpr{cc}, true
+		}
+		return cc, true
+	case "is-vendor-board-qcom":
+		if s, ok := maybeString(value); !ok || s != "" {
+			return ctx.newBadExpr(directive,
+				fmt.Sprintf("the result of %s can be compared only to empty", call.name)), true
+		}
+		// if the expression is ifneq (,$(call is-vendor-board-platform,...)), negate==true,
+		// so we should set inExpr.isNot to false
+		return &inExpr{
+			expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+			list:  &variableRefExpr{ctx.addVariable("QCOM_BOARD_PLATFORMS"), true},
+			isNot: isEq,
+		}, true
 	}
+	return nil, false
 }
 
 func (ctx *parseContext) parseCompareFilterFuncResult(cond *mkparser.Directive,

mk2rbc/mk2rbc_test.go

@@ -350,6 +350,21 @@ def init(g, handle):
     cfg["PRODUCT_MODEL"] = "pix21"
   if "aosp_x86" != g["TARGET_PRODUCT"]:
     cfg["PRODUCT_MODEL"] = "pix3"
+`,
+	},
+	{
+		desc:   "ifeq with soong_config_get",
+		mkname: "product.mk",
+		in: `
+ifeq (true,$(call soong_config_get,art_module,source_build))
+endif
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  if "true" == rblf.soong_config_get(g, "art_module", "source_build"):
+    pass
 `,
 	},
 	{
@@ -512,6 +527,21 @@ def init(g, handle):
     pass
   if rblf.file_wildcard_exists("foo*.mk"):
     pass
+`,
+	},
+	{
+		desc:   "if with interpolation",
+		mkname: "product.mk",
+		in: `
+ifeq ($(VARIABLE1)text$(VARIABLE2),true)
+endif
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  if "%stext%s" % (g.get("VARIABLE1", ""), g.get("VARIABLE2", "")) == "true":
+    pass
 `,
 	},
 	{