From 9932f75151c2eb6ac3069496cb9596292d0af797 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 8 Feb 2022 11:56:25 -0800 Subject: [PATCH] Allow filter calls with a list as a pattern This commit doesn't attempt to replace the filter calls with anything more idomatic for now. It also removes the case for filter-out, because the actual function name is filter_out and wasn't being used anyways. Even if I were to change it to filter_out, that would produce buggy results: ifneq (,$(filter $(TARGET_BUILD_VARIANT), userdebug eng)) endif ifneq (,$(filter-out $(TARGET_BUILD_VARIANT), userdebug eng)) endif Both of these would produce: if g["TARGET_BUILD_VARIANT"] in ["userdebug", "eng"]: pass Fixes: 218702402 Test: go test Change-Id: I566079e5d3a364c42db14045aa1bab9d99eba05f --- mk2rbc/mk2rbc.go | 77 ++++++++++++++----------------------------- mk2rbc/mk2rbc_test.go | 8 +++++ 2 files changed, 32 insertions(+), 53 deletions(-) diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go index b8fe16239..cb50a5014 100644 --- a/mk2rbc/mk2rbc.go +++ b/mk2rbc/mk2rbc.go @@ -1092,7 +1092,7 @@ func (ctx *parseContext) parseCompare(cond *mkparser.Directive) starlarkExpr { // Given an if statement's directive and the left/right starlarkExprs, // check if the starlarkExprs are one of a few hardcoded special cases -// that can be converted to a simpler equalify expression than simply comparing +// that can be converted to a simpler equality expression than simply comparing // the two. func (ctx *parseContext) parseCompareSpecialCases(directive *mkparser.Directive, left starlarkExpr, right starlarkExpr) (starlarkExpr, bool) { @@ -1121,8 +1121,8 @@ func (ctx *parseContext) parseCompareSpecialCases(directive *mkparser.Directive, } switch call.name { - case baseName + ".filter", baseName + ".filter-out": - return ctx.parseCompareFilterFuncResult(directive, call, value, isEq), true + case baseName + ".filter": + return ctx.parseCompareFilterFuncResult(directive, call, value, isEq) case baseName + ".expand_wildcard": return ctx.parseCompareWildcardFuncResult(directive, call, value, !isEq), true case baseName + ".findstring": @@ -1134,68 +1134,39 @@ func (ctx *parseContext) parseCompareSpecialCases(directive *mkparser.Directive, } func (ctx *parseContext) parseCompareFilterFuncResult(cond *mkparser.Directive, - filterFuncCall *callExpr, xValue starlarkExpr, negate bool) starlarkExpr { + filterFuncCall *callExpr, xValue starlarkExpr, negate bool) (starlarkExpr, bool) { // We handle: // * ifeq/ifneq (,$(filter v1 v2 ..., EXPR) becomes if EXPR not in/in ["v1", "v2", ...] // * ifeq/ifneq (,$(filter EXPR, v1 v2 ...) becomes if EXPR not in/in ["v1", "v2", ...] - // * ifeq/ifneq ($(VAR),$(filter $(VAR), v1 v2 ...) becomes if VAR in/not in ["v1", "v2"] - // TODO(Asmundak): check the last case works for filter-out, too. + if x, ok := xValue.(*stringLiteralExpr); !ok || x.literal != "" { + return nil, false + } xPattern := filterFuncCall.args[0] xText := filterFuncCall.args[1] var xInList *stringLiteralExpr var expr starlarkExpr var ok bool - switch x := xValue.(type) { - case *stringLiteralExpr: - if x.literal != "" { - return ctx.newBadExpr(cond, "filter comparison to non-empty value: %s", xValue) - } - // Either pattern or text should be const, and the - // non-const one should be varRefExpr - if xInList, ok = xPattern.(*stringLiteralExpr); ok && !strings.ContainsRune(xInList.literal, '%') && xText.typ() == starlarkTypeList { - expr = xText - } else if xInList, ok = xText.(*stringLiteralExpr); ok { - expr = xPattern - } else { - expr = &callExpr{ - object: nil, - name: filterFuncCall.name, - args: filterFuncCall.args, - returnType: starlarkTypeBool, - } - if negate { - expr = ¬Expr{expr: expr} - } - return expr - } - case *variableRefExpr: - if v, ok := xPattern.(*variableRefExpr); ok { - if xInList, ok = xText.(*stringLiteralExpr); ok && v.ref.name() == x.ref.name() { - // ifeq/ifneq ($(VAR),$(filter $(VAR), v1 v2 ...), flip negate, - // it's the opposite to what is done when comparing to empty. - expr = xPattern - negate = !negate - } - } + if xInList, ok = xPattern.(*stringLiteralExpr); ok && !strings.ContainsRune(xInList.literal, '%') && xText.typ() == starlarkTypeList { + expr = xText + } else if xInList, ok = xText.(*stringLiteralExpr); ok { + expr = xPattern + } else { + return nil, false } - if expr != nil && xInList != nil { - slExpr := newStringListExpr(strings.Fields(xInList.literal)) - // Generate simpler code for the common cases: - if expr.typ() == starlarkTypeList { - if len(slExpr.items) == 1 { - // Checking that a string belongs to list - return &inExpr{isNot: negate, list: expr, expr: slExpr.items[0]} - } else { - // TODO(asmundak): - panic("TBD") - } - } else if len(slExpr.items) == 1 { - return &eqExpr{left: expr, right: slExpr.items[0], isEq: !negate} + slExpr := newStringListExpr(strings.Fields(xInList.literal)) + // Generate simpler code for the common cases: + if expr.typ() == starlarkTypeList { + if len(slExpr.items) == 1 { + // Checking that a string belongs to list + return &inExpr{isNot: negate, list: expr, expr: slExpr.items[0]}, true } else { - return &inExpr{isNot: negate, list: newStringListExpr(strings.Fields(xInList.literal)), expr: expr} + return nil, false } + } else if len(slExpr.items) == 1 { + return &eqExpr{left: expr, right: slExpr.items[0], isEq: !negate}, true + } else { + return &inExpr{isNot: negate, list: newStringListExpr(strings.Fields(xInList.literal)), expr: expr}, true } - return ctx.newBadExpr(cond, "filter arguments are too complex: %s", cond.Dump()) } func (ctx *parseContext) parseCompareWildcardFuncResult(directive *mkparser.Directive, diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go index 2083121ab..55e37fef0 100644 --- a/mk2rbc/mk2rbc_test.go +++ b/mk2rbc/mk2rbc_test.go @@ -389,6 +389,10 @@ ifneq (,$(filter plaf,$(PLATFORM_LIST))) endif ifeq ($(TARGET_BUILD_VARIANT), $(filter $(TARGET_BUILD_VARIANT), userdebug eng)) endif +ifneq (, $(filter $(TARGET_BUILD_VARIANT), userdebug eng)) +endif +ifneq (,$(filter userdebug eng, $(TARGET_BUILD_VARIANT))) +endif ifneq (,$(filter true, $(v1)$(v2))) endif ifeq (,$(filter barbet coral%,$(TARGET_PRODUCT))) @@ -407,8 +411,12 @@ def init(g, handle): pass if "plaf" in g.get("PLATFORM_LIST", []): pass + if g["TARGET_BUILD_VARIANT"] == " ".join(rblf.filter(g["TARGET_BUILD_VARIANT"], "userdebug eng")): + pass if g["TARGET_BUILD_VARIANT"] in ["userdebug", "eng"]: pass + if rblf.filter("userdebug eng", g["TARGET_BUILD_VARIANT"]): + pass if rblf.filter("true", "%s%s" % (_v1, _v2)): pass if not rblf.filter("barbet coral%", g["TARGET_PRODUCT"]):