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
diff --git a/mk2rbc/expr.go b/mk2rbc/expr.go
index 54bb6d1..9266520 100644
--- a/mk2rbc/expr.go
+++ b/mk2rbc/expr.go
@@ -232,19 +232,18 @@
}
type variableRefExpr struct {
- ref variable
- isDefined bool
+ ref variable
}
-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 {
diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go
index 8807437..4cde7d1 100644
--- a/mk2rbc/mk2rbc.go
+++ b/mk2rbc/mk2rbc.go
@@ -195,17 +195,57 @@
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
- starScript *StarlarkScript
- indentLevel int
- inAssignment bool
- tracedCount int
+ buf strings.Builder
+ starScript *StarlarkScript
+ indentLevel int
+ inAssignment bool
+ 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 @@
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 @@
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 @@
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 @@
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 @@
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) 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 @@
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 = ¬Expr{v}
}
@@ -1280,12 +1280,12 @@
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 @@
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 @@
return ctx.newBadExpr(node, "cannot handle non-constant argument to is-vendor-board-platform")
}
return &inExpr{
- expr: &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
- list: &variableRefExpr{ctx.addVariable(args.Dump() + "_BOARD_PLATFORMS"), true},
+ expr: NewVariableRefExpr(ctx.addVariable("TARGET_BOARD_PLATFORM")),
+ list: NewVariableRefExpr(ctx.addVariable(args.Dump() + "_BOARD_PLATFORMS")),
isNot: false,
}
}
@@ -1407,8 +1407,8 @@
return ctx.newBadExpr(node, "is-vendor-board-qcom does not accept any arguments")
}
return &inExpr{
- expr: &variableRefExpr{ctx.addVariable("TARGET_BOARD_PLATFORM"), false},
- list: &variableRefExpr{ctx.addVariable("QCOM_BOARD_PLATFORMS"), true},
+ expr: NewVariableRefExpr(ctx.addVariable("TARGET_BOARD_PLATFORM")),
+ list: NewVariableRefExpr(ctx.addVariable("QCOM_BOARD_PLATFORMS")),
isNot: false,
}
}
diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go
index 3698813..33961f1 100644
--- a/mk2rbc/mk2rbc_test.go
+++ b/mk2rbc/mk2rbc_test.go
@@ -626,7 +626,7 @@
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 @@
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
`,
},
diff --git a/mk2rbc/node.go b/mk2rbc/node.go
index c0c4c98..7c39b9e 100644
--- a/mk2rbc/node.go
+++ b/mk2rbc/node.go
@@ -196,7 +196,6 @@
flavor assignmentFlavor
location ErrorLocation
isTraced bool
- previous *assignmentNode
}
func (asgn *assignmentNode) emit(gctx *generationContext) {
@@ -209,7 +208,7 @@
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 @@
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 @@
gctx.emitPass()
}
gctx.indentLevel--
+ gctx.popVariableAssignments()
}
// A single complete if ... elseif ... else ... endif sequences
@@ -302,6 +303,7 @@
}
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 @@
gctx.emitPass()
}
gctx.indentLevel--
+ gctx.popVariableAssignments()
}
diff --git a/mk2rbc/variable.go b/mk2rbc/variable.go
index be1b174..0a26ed8 100644
--- a/mk2rbc/variable.go
+++ b/mk2rbc/variable.go
@@ -21,9 +21,8 @@
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 @@
func (pcv productConfigVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
emitAssignment := func() {
- pcv.emitGet(gctx, true)
- gctx.write(" = ")
+ gctx.writef("cfg[%q] = ", pcv.nam)
asgn.value.emitListVarCopy(gctx)
}
emitAppend := func() {
- pcv.emitGet(gctx, true)
- gctx.write(" += ")
+ gctx.writef("cfg[%q] += ", pcv.nam)
value := asgn.value
if pcv.valueType() == starlarkTypeString {
gctx.writef(`" " + `)
@@ -98,7 +95,7 @@
}
// 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 @@
emitAssignment()
gctx.indentLevel--
}
+
+ gctx.setHasBeenAssigned(&pcv)
}
-func (pcv productConfigVariable) emitGet(gctx *generationContext, isDefined bool) {
- if isDefined || pcv.isPreset() {
+func (pcv productConfigVariable) emitGet(gctx *generationContext) {
+ 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.write(" = ")
+ gctx.writef("g[%q] = ", scv.nam)
asgn.value.emitListVarCopy(gctx)
}
emitAppend := func() {
- scv.emitGet(gctx, true)
- gctx.write(" += ")
+ gctx.writef("g[%q] += ", scv.nam)
value := asgn.value
if scv.valueType() == starlarkTypeString {
gctx.writef(`" " + `)
@@ -158,7 +151,7 @@
}
// 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 @@
emitAssignment()
gctx.indentLevel--
}
+
+ gctx.setHasBeenAssigned(&scv)
}
-func (scv otherGlobalVariable) emitGet(gctx *generationContext, isDefined bool) {
- if isDefined || scv.isPreset() {
+func (scv otherGlobalVariable) emitGet(gctx *generationContext) {
+ 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 @@
gctx.writef("%s = ", lv)
asgn.value.emitListVarCopy(gctx)
case asgnAppend:
- lv.emitGet(gctx, false)
- gctx.write(" += ")
+ gctx.writef("%s += ", lv)
value := asgn.value
if lv.valueType() == starlarkTypeString {
gctx.writef(`" " + `)
@@ -227,7 +213,7 @@
}
}
-func (lv localVariable) emitGet(gctx *generationContext, _ bool) {
+func (lv localVariable) emitGet(gctx *generationContext) {
gctx.writef("%s", lv)
}
@@ -236,7 +222,7 @@
value starlarkExpr
}
-func (pv predefinedVariable) emitGet(gctx *generationContext, _ bool) {
+func (pv predefinedVariable) emitGet(gctx *generationContext) {
pv.value.emit(gctx)
}
@@ -257,10 +243,6 @@
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",