Move variable assignment handling to generation context

This allows the parsing code to be cleaner, as it
doesn't have to care about variable assignments.

Bug: 228518745
Test: go test
Change-Id: I33425c2fb51acab4901bfa82a53d337b75210f8e

mk2rbc/expr.go

@@ -232,19 +232,18 @@ func (xi *interpolateExpr) transform(transformer func(expr starlarkExpr) starlar
 }
 
 type variableRefExpr struct {
-	ref       variable
+	ref variable
-	isDefined bool
 }
 
-func NewVariableRefExpr(ref variable, isDefined bool) starlarkExpr {
+func NewVariableRefExpr(ref variable) starlarkExpr {
 	if predefined, ok := ref.(*predefinedVariable); ok {
 		return predefined.value
 	}
-	return &variableRefExpr{ref, isDefined}
+	return &variableRefExpr{ref}
 }
 
 func (v *variableRefExpr) emit(gctx *generationContext) {
-	v.ref.emitGet(gctx, v.isDefined)
+	v.ref.emitGet(gctx)
 }
 
 func (v *variableRefExpr) typ() starlarkType {

mk2rbc/mk2rbc.go

@@ -195,17 +195,57 @@ func isMakeControlFunc(s string) bool {
 	return s == "error" || s == "warning" || s == "info"
 }
 
+// varAssignmentScope points to the last assignment for each variable
+// in the current block. It is used during the parsing to chain
+// the assignments to a variable together.
+type varAssignmentScope struct {
+	outer *varAssignmentScope
+	vars  map[string]bool
+}
+
 // Starlark output generation context
 type generationContext struct {
-	buf          strings.Builder
+	buf            strings.Builder
-	starScript   *StarlarkScript
+	starScript     *StarlarkScript
-	indentLevel  int
+	indentLevel    int
-	inAssignment bool
+	inAssignment   bool
-	tracedCount  int
+	tracedCount    int
+	varAssignments *varAssignmentScope
 }
 
 func NewGenerateContext(ss *StarlarkScript) *generationContext {
-	return &generationContext{starScript: ss}
+	return &generationContext{
+		starScript: ss,
+		varAssignments: &varAssignmentScope{
+			outer: nil,
+			vars:  make(map[string]bool),
+		},
+	}
+}
+
+func (gctx *generationContext) pushVariableAssignments() {
+	va := &varAssignmentScope{
+		outer: gctx.varAssignments,
+		vars:  make(map[string]bool),
+	}
+	gctx.varAssignments = va
+}
+
+func (gctx *generationContext) popVariableAssignments() {
+	gctx.varAssignments = gctx.varAssignments.outer
+}
+
+func (gctx *generationContext) hasBeenAssigned(v variable) bool {
+	for va := gctx.varAssignments; va != nil; va = va.outer {
+		if _, ok := va.vars[v.name()]; ok {
+			return true
+		}
+	}
+	return false
+}
+
+func (gctx *generationContext) setHasBeenAssigned(v variable) {
+	gctx.varAssignments.vars[v.name()] = true
 }
 
 // emit returns generated script
@@ -392,14 +432,6 @@ type StarlarkScript struct {
 	nodeLocator    func(pos mkparser.Pos) int
 }
 
-// varAssignmentScope points to the last assignment for each variable
-// in the current block. It is used during the parsing to chain
-// the assignments to a variable together.
-type varAssignmentScope struct {
-	outer *varAssignmentScope
-	vars  map[string]*assignmentNode
-}
-
 // parseContext holds the script we are generating and all the ephemeral data
 // needed during the parsing.
 type parseContext struct {
@@ -413,7 +445,6 @@ type parseContext struct {
 	errorLogger      ErrorLogger
 	tracedVariables  map[string]bool // variables to be traced in the generated script
 	variables        map[string]variable
-	varAssignments   *varAssignmentScope
 	outputDir        string
 	dependentModules map[string]*moduleInfo
 	soongNamespaces  map[string]map[string]bool
@@ -466,7 +497,6 @@ func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
 		typeHints:        make(map[string]starlarkType),
 		atTopOfMakefile:  true,
 	}
-	ctx.pushVarAssignments()
 	for _, item := range predefined {
 		ctx.variables[item.name] = &predefinedVariable{
 			baseVariable: baseVariable{nam: item.name, typ: starlarkTypeString},
@@ -477,31 +507,6 @@ func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
 	return ctx
 }
 
-func (ctx *parseContext) lastAssignment(v variable) *assignmentNode {
-	for va := ctx.varAssignments; va != nil; va = va.outer {
-		if v, ok := va.vars[v.name()]; ok {
-			return v
-		}
-	}
-	return nil
-}
-
-func (ctx *parseContext) setLastAssignment(v variable, asgn *assignmentNode) {
-	ctx.varAssignments.vars[v.name()] = asgn
-}
-
-func (ctx *parseContext) pushVarAssignments() {
-	va := &varAssignmentScope{
-		outer: ctx.varAssignments,
-		vars:  make(map[string]*assignmentNode),
-	}
-	ctx.varAssignments = va
-}
-
-func (ctx *parseContext) popVarAssignments() {
-	ctx.varAssignments = ctx.varAssignments.outer
-}
-
 func (ctx *parseContext) hasNodes() bool {
 	return ctx.currentNodeIndex < len(ctx.nodes)
 }
@@ -583,8 +588,6 @@ func (ctx *parseContext) handleAssignment(a *mkparser.Assignment) []starlarkNode
 		asgn.value = &toStringExpr{expr: asgn.value}
 	}
 
-	asgn.previous = ctx.lastAssignment(lhs)
-	ctx.setLastAssignment(lhs, asgn)
 	switch a.Type {
 	case "=", ":=":
 		asgn.flavor = asgnSet
@@ -939,11 +942,8 @@ func (ctx *parseContext) handleIfBlock(ifDirective *mkparser.Directive) starlark
 func (ctx *parseContext) processBranch(check *mkparser.Directive) *switchCase {
 	block := &switchCase{gate: ctx.parseCondition(check)}
 	defer func() {
-		ctx.popVarAssignments()
 		ctx.ifNestLevel--
-
 	}()
-	ctx.pushVarAssignments()
 	ctx.ifNestLevel++
 
 	for ctx.hasNodes() {
@@ -967,7 +967,7 @@ func (ctx *parseContext) parseCondition(check *mkparser.Directive) starlarkNode
 		if !check.Args.Const() {
 			return ctx.newBadNode(check, "ifdef variable ref too complex: %s", check.Args.Dump())
 		}
-		v := NewVariableRefExpr(ctx.addVariable(check.Args.Strings[0]), false)
+		v := NewVariableRefExpr(ctx.addVariable(check.Args.Strings[0]))
 		if strings.HasSuffix(check.Name, "ndef") {
 			v = &notExpr{v}
 		}
@@ -1280,12 +1280,12 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt
 				args: []starlarkExpr{
 					&stringLiteralExpr{literal: substParts[0]},
 					&stringLiteralExpr{literal: substParts[1]},
-					NewVariableRefExpr(v, ctx.lastAssignment(v) != nil),
+					NewVariableRefExpr(v),
 				},
 			}
 		}
 		if v := ctx.addVariable(refDump); v != nil {
-			return NewVariableRefExpr(v, ctx.lastAssignment(v) != nil)
+			return NewVariableRefExpr(v)
 		}
 		return ctx.newBadExpr(node, "unknown variable %s", refDump)
 	}
@@ -1381,7 +1381,7 @@ func (p *isProductInListCallParser) parse(ctx *parseContext, node mkparser.Node,
 		return ctx.newBadExpr(node, "is-product-in-list requires an argument")
 	}
 	return &inExpr{
-		expr:  &variableRefExpr{ctx.addVariable("TARGET_PRODUCT"), true},
+		expr:  NewVariableRefExpr(ctx.addVariable("TARGET_PRODUCT")),
 		list:  maybeConvertToStringList(ctx.parseMakeString(node, args)),
 		isNot: false,
 	}
@@ -1394,8 +1394,8 @@ func (p *isVendorBoardPlatformCallParser) parse(ctx *parseContext, node mkparser
 		return ctx.newBadExpr(node, "cannot handle non-constant argument to is-vendor-board-platform")
 	}
 	return &inExpr{
-		expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+		expr:  NewVariableRefExpr(ctx.addVariable("TARGET_BOARD_PLATFORM")),
-		list:  &variableRefExpr{ctx.addVariable(args.Dump() + "_BOARD_PLATFORMS"), true},
+		list:  NewVariableRefExpr(ctx.addVariable(args.Dump() + "_BOARD_PLATFORMS")),
 		isNot: false,
 	}
 }
@@ -1407,8 +1407,8 @@ func (p *isVendorBoardQcomCallParser) parse(ctx *parseContext, node mkparser.Nod
 		return ctx.newBadExpr(node, "is-vendor-board-qcom does not accept any arguments")
 	}
 	return &inExpr{
-		expr:  &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
+		expr:  NewVariableRefExpr(ctx.addVariable("TARGET_BOARD_PLATFORM")),
-		list:  &variableRefExpr{ctx.addVariable("QCOM_BOARD_PLATFORMS"), true},
+		list:  NewVariableRefExpr(ctx.addVariable("QCOM_BOARD_PLATFORMS")),
 		isNot: false,
 	}
 }

mk2rbc/mk2rbc_test.go

@@ -626,7 +626,7 @@ def init(g, handle):
     pass
   elif not rblf.board_platform_is(g, "copper"):
     pass
-  elif g.get("TARGET_BOARD_PLATFORM", "") not in g["QCOM_BOARD_PLATFORMS"]:
+  elif g.get("TARGET_BOARD_PLATFORM", "") not in g.get("QCOM_BOARD_PLATFORMS", ""):
     pass
   elif g["TARGET_PRODUCT"] in g.get("PLATFORM_LIST", []):
     pass
@@ -649,7 +649,7 @@ def init(g, handle):
     pass
   elif not rblf.board_platform_is(g, "copper"):
     pass
-  elif g.get("TARGET_BOARD_PLATFORM", "") in g["QCOM_BOARD_PLATFORMS"]:
+  elif g.get("TARGET_BOARD_PLATFORM", "") in g.get("QCOM_BOARD_PLATFORMS", ""):
     pass
 `,
 	},

mk2rbc/node.go

@@ -196,7 +196,6 @@ type assignmentNode struct {
 	flavor   assignmentFlavor
 	location ErrorLocation
 	isTraced bool
-	previous *assignmentNode
 }
 
 func (asgn *assignmentNode) emit(gctx *generationContext) {
@@ -209,7 +208,7 @@ func (asgn *assignmentNode) emit(gctx *generationContext) {
 		gctx.newLine()
 		gctx.tracedCount++
 		gctx.writef(`print("%s.%d: %s := ", `, gctx.starScript.mkFile, gctx.tracedCount, asgn.lhs.name())
-		asgn.lhs.emitGet(gctx, true)
+		asgn.lhs.emitGet(gctx)
 		gctx.writef(")")
 	}
 }
@@ -271,6 +270,7 @@ type switchCase struct {
 func (cb *switchCase) emit(gctx *generationContext) {
 	cb.gate.emit(gctx)
 	gctx.indentLevel++
+	gctx.pushVariableAssignments()
 	hasStatements := false
 	for _, node := range cb.nodes {
 		if _, ok := node.(*commentNode); !ok {
@@ -282,6 +282,7 @@ func (cb *switchCase) emit(gctx *generationContext) {
 		gctx.emitPass()
 	}
 	gctx.indentLevel--
+	gctx.popVariableAssignments()
 }
 
 // A single complete if ... elseif ... else ... endif sequences
@@ -302,6 +303,7 @@ type foreachNode struct {
 }
 
 func (f *foreachNode) emit(gctx *generationContext) {
+	gctx.pushVariableAssignments()
 	gctx.newLine()
 	gctx.writef("for %s in ", f.varName)
 	f.list.emit(gctx)
@@ -318,4 +320,5 @@ func (f *foreachNode) emit(gctx *generationContext) {
 		gctx.emitPass()
 	}
 	gctx.indentLevel--
+	gctx.popVariableAssignments()
 }

mk2rbc/variable.go

@@ -21,9 +21,8 @@ import (
 
 type variable interface {
 	name() string
-	emitGet(gctx *generationContext, isDefined bool)
+	emitGet(gctx *generationContext)
 	emitSet(gctx *generationContext, asgn *assignmentNode)
-	emitDefined(gctx *generationContext)
 	valueType() starlarkType
 	setValueType(t starlarkType)
 	defaultValueString() string
@@ -74,13 +73,11 @@ type productConfigVariable struct {
 
 func (pcv productConfigVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
 	emitAssignment := func() {
-		pcv.emitGet(gctx, true)
+		gctx.writef("cfg[%q] = ", pcv.nam)
-		gctx.write(" = ")
 		asgn.value.emitListVarCopy(gctx)
 	}
 	emitAppend := func() {
-		pcv.emitGet(gctx, true)
+		gctx.writef("cfg[%q] += ", pcv.nam)
-		gctx.write(" += ")
 		value := asgn.value
 		if pcv.valueType() == starlarkTypeString {
 			gctx.writef(`" " + `)
@@ -98,7 +95,7 @@ func (pcv productConfigVariable) emitSet(gctx *generationContext, asgn *assignme
 	}
 
 	// If we are not sure variable has been assigned before, emit setdefault
-	needsSetDefault := asgn.previous == nil && !pcv.isPreset() && asgn.isSelfReferential()
+	needsSetDefault := !gctx.hasBeenAssigned(&pcv) && !pcv.isPreset() && asgn.isSelfReferential()
 
 	switch asgn.flavor {
 	case asgnSet:
@@ -121,34 +118,30 @@ func (pcv productConfigVariable) emitSet(gctx *generationContext, asgn *assignme
 		emitAssignment()
 		gctx.indentLevel--
 	}
+
+	gctx.setHasBeenAssigned(&pcv)
 }
 
-func (pcv productConfigVariable) emitGet(gctx *generationContext, isDefined bool) {
+func (pcv productConfigVariable) emitGet(gctx *generationContext) {
-	if isDefined || pcv.isPreset() {
+	if gctx.hasBeenAssigned(&pcv) || pcv.isPreset() {
 		gctx.writef("cfg[%q]", pcv.nam)
 	} else {
 		gctx.writef("cfg.get(%q, %s)", pcv.nam, pcv.defaultValueString())
 	}
 }
 
-func (pcv productConfigVariable) emitDefined(gctx *generationContext) {
-	gctx.writef("cfg.get(%q) != None", pcv.name())
-}
-
 type otherGlobalVariable struct {
 	baseVariable
 }
 
 func (scv otherGlobalVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
 	emitAssignment := func() {
-		scv.emitGet(gctx, true)
+		gctx.writef("g[%q] = ", scv.nam)
-		gctx.write(" = ")
 		asgn.value.emitListVarCopy(gctx)
 	}
 
 	emitAppend := func() {
-		scv.emitGet(gctx, true)
+		gctx.writef("g[%q] += ", scv.nam)
-		gctx.write(" += ")
 		value := asgn.value
 		if scv.valueType() == starlarkTypeString {
 			gctx.writef(`" " + `)
@@ -158,7 +151,7 @@ func (scv otherGlobalVariable) emitSet(gctx *generationContext, asgn *assignment
 	}
 
 	// If we are not sure variable has been assigned before, emit setdefault
-	needsSetDefault := asgn.previous == nil && !scv.isPreset() && asgn.isSelfReferential()
+	needsSetDefault := !gctx.hasBeenAssigned(&scv) && !scv.isPreset() && asgn.isSelfReferential()
 
 	switch asgn.flavor {
 	case asgnSet:
@@ -184,28 +177,22 @@ func (scv otherGlobalVariable) emitSet(gctx *generationContext, asgn *assignment
 		emitAssignment()
 		gctx.indentLevel--
 	}
+
+	gctx.setHasBeenAssigned(&scv)
 }
 
-func (scv otherGlobalVariable) emitGet(gctx *generationContext, isDefined bool) {
+func (scv otherGlobalVariable) emitGet(gctx *generationContext) {
-	if isDefined || scv.isPreset() {
+	if gctx.hasBeenAssigned(&scv) || scv.isPreset() {
 		gctx.writef("g[%q]", scv.nam)
 	} else {
 		gctx.writef("g.get(%q, %s)", scv.nam, scv.defaultValueString())
 	}
 }
 
-func (scv otherGlobalVariable) emitDefined(gctx *generationContext) {
-	gctx.writef("g.get(%q) != None", scv.name())
-}
-
 type localVariable struct {
 	baseVariable
 }
 
-func (lv localVariable) emitDefined(gctx *generationContext) {
-	gctx.writef(lv.String())
-}
-
 func (lv localVariable) String() string {
 	return "_" + lv.nam
 }
@@ -216,8 +203,7 @@ func (lv localVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
 		gctx.writef("%s = ", lv)
 		asgn.value.emitListVarCopy(gctx)
 	case asgnAppend:
-		lv.emitGet(gctx, false)
+		gctx.writef("%s += ", lv)
-		gctx.write(" += ")
 		value := asgn.value
 		if lv.valueType() == starlarkTypeString {
 			gctx.writef(`" " + `)
@@ -227,7 +213,7 @@ func (lv localVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
 	}
 }
 
-func (lv localVariable) emitGet(gctx *generationContext, _ bool) {
+func (lv localVariable) emitGet(gctx *generationContext) {
 	gctx.writef("%s", lv)
 }
 
@@ -236,7 +222,7 @@ type predefinedVariable struct {
 	value starlarkExpr
 }
 
-func (pv predefinedVariable) emitGet(gctx *generationContext, _ bool) {
+func (pv predefinedVariable) emitGet(gctx *generationContext) {
 	pv.value.emit(gctx)
 }
 
@@ -257,10 +243,6 @@ func (pv predefinedVariable) emitSet(gctx *generationContext, asgn *assignmentNo
 	panic(fmt.Errorf("cannot set predefined variable %s to %q", pv.name(), asgn.mkValue.Dump()))
 }
 
-func (pv predefinedVariable) emitDefined(gctx *generationContext) {
-	gctx.write("True")
-}
-
 var localProductConfigVariables = map[string]string{
 	"LOCAL_AUDIO_PRODUCT_PACKAGE":         "PRODUCT_PACKAGES",
 	"LOCAL_AUDIO_PRODUCT_COPY_FILES":      "PRODUCT_COPY_FILES",