Use strings instead of simpleNinjaStrings where possible

Storing every string without ninja variable references through
simpleNinjaString costs 24 bytes and a heap allocation.  16 bytes
is used for the ninjaString.str string, 8 bytes for the
ninjaString.variables *[]variableReference.  An additional 8 bytes
is used for the resulting pointer into the heap.

The vast majority of calls to simpleNinjaString originate in
blueprint.parseBuildParams, which converts all of the parameters
passed to ctx.Build into ninjaStrings.  All together this was
allocating 1.575 GB of *ninjaString objects.

Add a parseNinjaOrSimpleStrings function that converts input strings
into ninjaStrings if they have ninja variable references, but also
returns a slice of plain strings for input strings without any ninja
variable references.  That still results in 1.39 GB of allocations just
for the output string slice, so also add an optimization that reuses
the input string slice as the output slice if all of the strings had
no variable references.

Plumb the resulting strings through everywhere that the []*ninjaStrings
were used.

This reduces the total memory allocations inside
blueprint.parseBuildParams in my AOSP aosp_cf_x86_64_phone-userdebug
build from 3.337 GB to 1.786 GB.

Test: ninja_strings_test.go
Change-Id: I51bc138a2a6b1cc7383c7df0a483ccb067ffa02b

bootstrap/command.go

@@ -135,7 +135,7 @@ func RunBlueprint(args Args, stopBefore StopBefore, ctx *blueprint.Context, conf
 	}
 
 	const outFilePermissions = 0666
-	var out io.StringWriter
+	var out blueprint.StringWriterWriter
 	var f *os.File
 	var buf *bufio.Writer
 
@@ -145,7 +145,7 @@ func RunBlueprint(args Args, stopBefore StopBefore, ctx *blueprint.Context, conf
 		if err := os.WriteFile(joinPath(ctx.SrcDir(), args.OutFile), []byte(nil), outFilePermissions); err != nil {
 			return nil, fmt.Errorf("error writing empty Ninja file: %s", err)
 		}
-		out = io.Discard.(io.StringWriter)
+		out = io.Discard.(blueprint.StringWriterWriter)
 	} else {
 		f, err := os.OpenFile(joinPath(ctx.SrcDir(), args.OutFile), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, outFilePermissions)
 		if err != nil {

context.go

@@ -2786,11 +2786,16 @@ func jsonModuleWithActionsFromModuleInfo(m *moduleInfo) *JsonModule {
 	var actions []JSONAction
 	for _, bDef := range m.actionDefs.buildDefs {
 		a := JSONAction{
-			Inputs: append(
+			Inputs: append(append(append(
-				getNinjaStringsWithNilPkgNames(bDef.Inputs),
+				bDef.InputStrings,
+				bDef.ImplicitStrings...),
+				getNinjaStringsWithNilPkgNames(bDef.Inputs)...),
 				getNinjaStringsWithNilPkgNames(bDef.Implicits)...),
-			Outputs: append(
+
-				getNinjaStringsWithNilPkgNames(bDef.Outputs),
+			Outputs: append(append(append(
+				bDef.OutputStrings,
+				bDef.ImplicitOutputStrings...),
+				getNinjaStringsWithNilPkgNames(bDef.Outputs)...),
 				getNinjaStringsWithNilPkgNames(bDef.ImplicitOutputs)...),
 		}
 		if d, ok := bDef.Variables["description"]; ok {
@@ -3992,6 +3997,9 @@ func (c *Context) AllTargets() (map[string]string, error) {
 				}
 				targets[outputValue] = ruleName
 			}
+			for _, output := range append(buildDef.OutputStrings, buildDef.ImplicitOutputStrings...) {
+				targets[output] = ruleName
+			}
 		}
 		return nil
 	}
@@ -4214,7 +4222,7 @@ func (c *Context) SingletonName(singleton Singleton) string {
 // WriteBuildFile writes the Ninja manifest text for the generated build
 // actions to w.  If this is called before PrepareBuildActions successfully
 // completes then ErrBuildActionsNotReady is returned.
-func (c *Context) WriteBuildFile(w io.StringWriter) error {
+func (c *Context) WriteBuildFile(w StringWriterWriter) error {
 	var err error
 	pprof.Do(c.Context, pprof.Labels("blueprint", "WriteBuildFile"), func(ctx context.Context) {
 		if !c.buildActionsReady {
@@ -4696,7 +4704,7 @@ type phonyCandidate struct {
 // keyForPhonyCandidate gives a unique identifier for a set of deps.
 // If any of the deps use a variable, we return an empty string to signal
 // that this set of deps is ineligible for extraction.
-func keyForPhonyCandidate(deps []*ninjaString) string {
+func keyForPhonyCandidate(deps []*ninjaString, stringDeps []string) string {
 	hasher := sha256.New()
 	for _, d := range deps {
 		if len(d.Variables()) != 0 {
@@ -4704,6 +4712,9 @@ func keyForPhonyCandidate(deps []*ninjaString) string {
 		}
 		io.WriteString(hasher, d.Value(nil))
 	}
+	for _, d := range stringDeps {
+		io.WriteString(hasher, d)
+	}
 	return base64.RawURLEncoding.EncodeToString(hasher.Sum(nil))
 }
 
@@ -4713,7 +4724,7 @@ func keyForPhonyCandidate(deps []*ninjaString) string {
 // (and phonyCandidate#first.OrderOnly) will be replaced with phonyCandidate#phony.Outputs
 func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.Uint32, b *buildDef) {
 	defer wg.Done()
-	key := keyForPhonyCandidate(b.OrderOnly)
+	key := keyForPhonyCandidate(b.OrderOnly, b.OrderOnlyStrings)
 	if key == "" {
 		return
 	}
@@ -4726,17 +4737,20 @@ func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.U
 			// extract it as a phony output
 			phonyCount.Add(1)
 			m.phony = &buildDef{
-				Rule:     Phony,
+				Rule:          Phony,
-				Outputs:  []*ninjaString{simpleNinjaString("dedup-" + key)},
+				OutputStrings: []string{"dedup-" + key},
-				Inputs:   m.first.OrderOnly, //we could also use b.OrderOnly
+				Inputs:        m.first.OrderOnly, //we could also use b.OrderOnly
-				Optional: true,
+				InputStrings:  m.first.OrderOnlyStrings,
+				Optional:      true,
 			}
 			// the previously recorded build-def, which first had these deps as its
 			// order-only deps, should now use this phony output instead
-			m.first.OrderOnly = m.phony.Outputs
+			m.first.OrderOnlyStrings = m.phony.OutputStrings
+			m.first.OrderOnly = nil
 			m.first = nil
 		})
-		b.OrderOnly = m.phony.Outputs
+		b.OrderOnlyStrings = m.phony.OutputStrings
+		b.OrderOnly = nil
 	}
 }
 
@@ -4753,7 +4767,7 @@ func (c *Context) deduplicateOrderOnlyDeps(infos []*moduleInfo) *localBuildActio
 	wg := sync.WaitGroup{}
 	for _, info := range infos {
 		for _, b := range info.actionDefs.buildDefs {
-			if len(b.OrderOnly) > 0 {
+			if len(b.OrderOnly) > 0 || len(b.OrderOnlyStrings) > 0 {
 				wg.Add(1)
 				go scanBuildDef(&wg, &candidates, &phonyCount, b)
 			}
@@ -4774,7 +4788,7 @@ func (c *Context) deduplicateOrderOnlyDeps(infos []*moduleInfo) *localBuildActio
 	c.EventHandler.Do("sort_phony_builddefs", func() {
 		// sorting for determinism, the phony output names are stable
 		sort.Slice(phonys, func(i int, j int) bool {
-			return phonys[i].Outputs[0].Value(nil) < phonys[j].Outputs[0].Value(nil)
+			return phonys[i].OutputStrings[0] < phonys[j].OutputStrings[0]
 		})
 	})
 

context_test.go

@@ -1159,18 +1159,11 @@ func TestPackageIncludes(t *testing.T) {
 }
 
 func TestDeduplicateOrderOnlyDeps(t *testing.T) {
-	outputs := func(names ...string) []*ninjaString {
-		r := make([]*ninjaString, len(names))
-		for i, name := range names {
-			r[i] = simpleNinjaString(name)
-		}
-		return r
-	}
 	b := func(output string, inputs []string, orderOnlyDeps []string) *buildDef {
 		return &buildDef{
-			Outputs:   outputs(output),
+			OutputStrings:    []string{output},
-			Inputs:    outputs(inputs...),
+			InputStrings:     inputs,
-			OrderOnly: outputs(orderOnlyDeps...),
+			OrderOnlyStrings: orderOnlyDeps,
 		}
 	}
 	m := func(bs ...*buildDef) *moduleInfo {
@@ -1179,7 +1172,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 	type testcase struct {
 		modules        []*moduleInfo
 		expectedPhonys []*buildDef
-		conversions    map[string][]*ninjaString
+		conversions    map[string][]string
 	}
 	testCases := []testcase{{
 		modules: []*moduleInfo{
@@ -1189,9 +1182,9 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 		expectedPhonys: []*buildDef{
 			b("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ", []string{"d"}, nil),
 		},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"),
+			"A": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
-			"B": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"),
+			"B": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1205,10 +1198,10 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 			m(b("C", nil, []string{"a"})),
 		},
 		expectedPhonys: []*buildDef{b("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs", []string{"a"}, nil)},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"),
+			"A": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
-			"B": outputs("b"),
+			"B": []string{"b"},
-			"C": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"),
+			"C": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1220,11 +1213,11 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 		expectedPhonys: []*buildDef{
 			b("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM", []string{"a", "b"}, nil),
 			b("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E", []string{"a", "c"}, nil)},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"),
+			"A": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"B": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"),
+			"B": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"C": outputs("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"),
+			"C": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
-			"D": outputs("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"),
+			"D": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
 		},
 	}}
 	for index, tc := range testCases {
@@ -1253,7 +1246,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 			find := func(k string) *buildDef {
 				for _, m := range tc.modules {
 					for _, b := range m.actionDefs.buildDefs {
-						if reflect.DeepEqual(b.Outputs, outputs(k)) {
+						if reflect.DeepEqual(b.OutputStrings, []string{k}) {
 							return b
 						}
 					}
@@ -1265,7 +1258,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 				if actual == nil {
 					t.Errorf("Couldn't find %s", k)
 				}
-				if !reflect.DeepEqual(actual.OrderOnly, conversion) {
+				if !reflect.DeepEqual(actual.OrderOnlyStrings, conversion) {
 					t.Errorf("expected %s.OrderOnly = %v but got %v", k, conversion, actual.OrderOnly)
 				}
 			}

go.mod

@@ -1,3 +1,3 @@
 module github.com/google/blueprint
 
-go 1.18
+go 1.20

ninja_defs.go

@@ -261,18 +261,24 @@ func (r *ruleDef) WriteTo(nw *ninjaWriter, name string,
 
 // A buildDef describes a build target definition.
 type buildDef struct {
-	Comment         string
+	Comment               string
-	Rule            Rule
+	Rule                  Rule
-	RuleDef         *ruleDef
+	RuleDef               *ruleDef
-	Outputs         []*ninjaString
+	Outputs               []*ninjaString
-	ImplicitOutputs []*ninjaString
+	OutputStrings         []string
-	Inputs          []*ninjaString
+	ImplicitOutputs       []*ninjaString
-	Implicits       []*ninjaString
+	ImplicitOutputStrings []string
-	OrderOnly       []*ninjaString
+	Inputs                []*ninjaString
-	Validations     []*ninjaString
+	InputStrings          []string
-	Args            map[Variable]*ninjaString
+	Implicits             []*ninjaString
-	Variables       map[string]*ninjaString
+	ImplicitStrings       []string
-	Optional        bool
+	OrderOnly             []*ninjaString
+	OrderOnlyStrings      []string
+	Validations           []*ninjaString
+	ValidationStrings     []string
+	Args                  map[Variable]*ninjaString
+	Variables             map[string]*ninjaString
+	Optional              bool
 }
 
 func formatTags(tags map[string]string, rule Rule) string {
@@ -319,32 +325,32 @@ func parseBuildParams(scope scope, params *BuildParams,
 	}
 
 	var err error
-	b.Outputs, err = parseNinjaStrings(scope, params.Outputs)
+	b.Outputs, b.OutputStrings, err = parseNinjaOrSimpleStrings(scope, params.Outputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Outputs param: %s", err)
 	}
 
-	b.ImplicitOutputs, err = parseNinjaStrings(scope, params.ImplicitOutputs)
+	b.ImplicitOutputs, b.ImplicitOutputStrings, err = parseNinjaOrSimpleStrings(scope, params.ImplicitOutputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing ImplicitOutputs param: %s", err)
 	}
 
-	b.Inputs, err = parseNinjaStrings(scope, params.Inputs)
+	b.Inputs, b.InputStrings, err = parseNinjaOrSimpleStrings(scope, params.Inputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Inputs param: %s", err)
 	}
 
-	b.Implicits, err = parseNinjaStrings(scope, params.Implicits)
+	b.Implicits, b.ImplicitStrings, err = parseNinjaOrSimpleStrings(scope, params.Implicits)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Implicits param: %s", err)
 	}
 
-	b.OrderOnly, err = parseNinjaStrings(scope, params.OrderOnly)
+	b.OrderOnly, b.OrderOnlyStrings, err = parseNinjaOrSimpleStrings(scope, params.OrderOnly)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing OrderOnly param: %s", err)
 	}
 
-	b.Validations, err = parseNinjaStrings(scope, params.Validations)
+	b.Validations, b.ValidationStrings, err = parseNinjaOrSimpleStrings(scope, params.Validations)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Validations param: %s", err)
 	}
@@ -411,14 +417,20 @@ func parseBuildParams(scope scope, params *BuildParams,
 
 func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) error {
 	var (
-		comment       = b.Comment
+		comment             = b.Comment
-		rule          = b.Rule.fullName(pkgNames)
+		rule                = b.Rule.fullName(pkgNames)
-		outputs       = b.Outputs
+		outputs             = b.Outputs
-		implicitOuts  = b.ImplicitOutputs
+		implicitOuts        = b.ImplicitOutputs
-		explicitDeps  = b.Inputs
+		explicitDeps        = b.Inputs
-		implicitDeps  = b.Implicits
+		implicitDeps        = b.Implicits
-		orderOnlyDeps = b.OrderOnly
+		orderOnlyDeps       = b.OrderOnly
-		validations   = b.Validations
+		validations         = b.Validations
+		outputStrings       = b.OutputStrings
+		implicitOutStrings  = b.ImplicitOutputStrings
+		explicitDepStrings  = b.InputStrings
+		implicitDepStrings  = b.ImplicitStrings
+		orderOnlyDepStrings = b.OrderOnlyStrings
+		validationStrings   = b.ValidationStrings
 	)
 
 	if b.RuleDef != nil {
@@ -426,7 +438,10 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 		orderOnlyDeps = append(b.RuleDef.CommandOrderOnly, orderOnlyDeps...)
 	}
 
-	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations, pkgNames)
+	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations,
+		outputStrings, implicitOutStrings, explicitDepStrings,
+		implicitDepStrings, orderOnlyDepStrings, validationStrings,
+		pkgNames)
 	if err != nil {
 		return err
 	}
@@ -456,7 +471,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	}
 
 	if !b.Optional {
-		err = nw.Default(pkgNames, outputs...)
+		err = nw.Default(pkgNames, outputs, outputStrings)
 		if err != nil {
 			return err
 		}

ninja_strings.go

@@ -87,16 +87,31 @@ func (ps *parseState) pushVariable(start, end int, v Variable) {
 type stateFunc func(*parseState, int, rune) (stateFunc, error)
 
 // parseNinjaString parses an unescaped ninja string (i.e. all $<something>
-// occurrences are expected to be variables or $$) and returns a list of the
+// occurrences are expected to be variables or $$) and returns a *ninjaString
-// variable names that the string references.
+// that contains the original string and a list of the referenced variables.
 func parseNinjaString(scope scope, str string) (*ninjaString, error) {
+	ninjaString, str, err := parseNinjaOrSimpleString(scope, str)
+	if err != nil {
+		return nil, err
+	}
+	if ninjaString != nil {
+		return ninjaString, nil
+	}
+	return simpleNinjaString(str), nil
+}
+
+// parseNinjaOrSimpleString parses an unescaped ninja string (i.e. all $<something>
+// occurrences are expected to be variables or $$) and returns either a *ninjaString
+// if the string contains ninja variable references, or the original string and nil
+// for the *ninjaString if it doesn't.
+func parseNinjaOrSimpleString(scope scope, str string) (*ninjaString, string, error) {
 	// naively pre-allocate slice by counting $ signs
 	n := strings.Count(str, "$")
 	if n == 0 {
 		if len(str) > 0 && str[0] == ' ' {
 			str = "$" + str
 		}
-		return simpleNinjaString(str), nil
+		return nil, str, nil
 	}
 	variableReferences := make([]variableReference, 0, n)
 	result := &ninjaString{
@@ -116,13 +131,13 @@ func parseNinjaString(scope scope, str string) (*ninjaString, error) {
 		r := rune(str[i])
 		state, err = state(parseState, i, r)
 		if err != nil {
-			return nil, fmt.Errorf("error parsing ninja string %q: %s", str, err)
+			return nil, "", fmt.Errorf("error parsing ninja string %q: %s", str, err)
 		}
 	}
 
 	_, err = state(parseState, len(parseState.str), eof)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	// All the '$' characters counted initially could have been "$$" escapes, leaving no
@@ -131,7 +146,7 @@ func parseNinjaString(scope scope, str string) (*ninjaString, error) {
 		result.variables = nil
 	}
 
-	return result, nil
+	return result, "", nil
 }
 
 func parseFirstRuneState(state *parseState, i int, r rune) (stateFunc, error) {
@@ -246,6 +261,8 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) {
 	}
 }
 
+// parseNinjaStrings converts a list of strings to *ninjaStrings by finding the references
+// to ninja variables contained in the strings.
 func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
 	error) {
 
@@ -263,6 +280,50 @@ func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
 	return result, nil
 }
 
+// parseNinjaOrSimpleStrings splits a list of strings into *ninjaStrings if they have ninja
+// variable references or a list of strings if they don't.  If none of the input strings contain
+// ninja variable references (a very common case) then it returns the unmodified input slice as
+// the output slice.
+func parseNinjaOrSimpleStrings(scope scope, strs []string) ([]*ninjaString, []string, error) {
+	if len(strs) == 0 {
+		return nil, strs, nil
+	}
+
+	// allSimpleStrings is true until the first time a string with ninja variable references is found.
+	allSimpleStrings := true
+	var simpleStrings []string
+	var ninjaStrings []*ninjaString
+
+	for i, str := range strs {
+		ninjaStr, simpleStr, err := parseNinjaOrSimpleString(scope, str)
+		if err != nil {
+			return nil, nil, fmt.Errorf("error parsing element %d: %s", i, err)
+		} else if ninjaStr != nil {
+			ninjaStrings = append(ninjaStrings, ninjaStr)
+			if allSimpleStrings && i > 0 {
+				// If all previous strings had no ninja variable references then they weren't copied into
+				// simpleStrings to avoid allocating it if the input slice is reused as the output.  Allocate
+				// simpleStrings and copy all the previous strings into it.
+				simpleStrings = make([]string, i, len(strs))
+				copy(simpleStrings, strs[:i])
+			}
+			allSimpleStrings = false
+		} else {
+			if !allSimpleStrings {
+				// Only copy into the output slice if at least one string with ninja variable references
+				// was found.  Skipped strings will be copied the first time a string with ninja variable
+				// is found.
+				simpleStrings = append(simpleStrings, simpleStr)
+			}
+		}
+	}
+	if allSimpleStrings {
+		// None of the input strings had ninja variable references, return the input slice as the output.
+		return nil, strs, nil
+	}
+	return ninjaStrings, simpleStrings, nil
+}
+
 func (n *ninjaString) Value(pkgNames map[*packageContext]string) string {
 	if n.variables == nil || len(*n.variables) == 0 {
 		return defaultEscaper.Replace(n.str)

ninja_strings_test.go

@@ -19,6 +19,7 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"unsafe"
 )
 
 type testVariableRef struct {
@@ -215,6 +216,105 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) {
 	}
 }
 
+func Test_parseNinjaOrSimpleStrings(t *testing.T) {
+	testCases := []struct {
+		name            string
+		in              []string
+		outStrings      []string
+		outNinjaStrings []string
+		sameSlice       bool
+	}{
+		{
+			name:      "nil",
+			in:        nil,
+			sameSlice: true,
+		},
+		{
+			name:      "empty",
+			in:        []string{},
+			sameSlice: true,
+		},
+		{
+			name:      "string",
+			in:        []string{"abc"},
+			sameSlice: true,
+		},
+		{
+			name:            "ninja string",
+			in:              []string{"$abc"},
+			outStrings:      nil,
+			outNinjaStrings: []string{"${abc}"},
+		},
+		{
+			name:            "ninja string first",
+			in:              []string{"$abc", "def", "ghi"},
+			outStrings:      []string{"def", "ghi"},
+			outNinjaStrings: []string{"${abc}"},
+		},
+		{
+			name:            "ninja string middle",
+			in:              []string{"abc", "$def", "ghi"},
+			outStrings:      []string{"abc", "ghi"},
+			outNinjaStrings: []string{"${def}"},
+		},
+		{
+			name:            "ninja string last",
+			in:              []string{"abc", "def", "$ghi"},
+			outStrings:      []string{"abc", "def"},
+			outNinjaStrings: []string{"${ghi}"},
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			inCopy := append([]string(nil), tt.in...)
+
+			scope := newLocalScope(nil, "")
+			scope.AddLocalVariable("abc", "abc")
+			scope.AddLocalVariable("def", "def")
+			scope.AddLocalVariable("ghi", "ghi")
+			gotNinjaStrings, gotStrings, err := parseNinjaOrSimpleStrings(scope, tt.in)
+			if err != nil {
+				t.Errorf("unexpected error %s", err)
+			}
+
+			wantStrings := tt.outStrings
+			if tt.sameSlice {
+				wantStrings = tt.in
+			}
+
+			wantNinjaStrings := tt.outNinjaStrings
+
+			var evaluatedNinjaStrings []string
+			if gotNinjaStrings != nil {
+				evaluatedNinjaStrings = make([]string, 0, len(gotNinjaStrings))
+				for _, ns := range gotNinjaStrings {
+					evaluatedNinjaStrings = append(evaluatedNinjaStrings, ns.Value(nil))
+				}
+			}
+
+			if !reflect.DeepEqual(gotStrings, wantStrings) {
+				t.Errorf("incorrect strings output, want %q got %q", wantStrings, gotStrings)
+			}
+			if !reflect.DeepEqual(evaluatedNinjaStrings, wantNinjaStrings) {
+				t.Errorf("incorrect ninja strings output, want %q got %q", wantNinjaStrings, evaluatedNinjaStrings)
+			}
+			if len(inCopy) != len(tt.in) && (len(tt.in) == 0 || !reflect.DeepEqual(inCopy, tt.in)) {
+				t.Errorf("input modified, want %#v, got %#v", inCopy, tt.in)
+			}
+
+			if (unsafe.SliceData(tt.in) == unsafe.SliceData(gotStrings)) != tt.sameSlice {
+				if tt.sameSlice {
+					t.Errorf("expected input and output slices to have the same backing arrays")
+				} else {
+					t.Errorf("expected input and output slices to have different backing arrays")
+				}
+			}
+
+		})
+	}
+}
+
 func Benchmark_parseNinjaString(b *testing.B) {
 	b.Run("constant", func(b *testing.B) {
 		for _, l := range []int{1, 10, 100, 1000} {

ninja_writer.go

@@ -34,12 +34,12 @@ type StringWriterWriter interface {
 }
 
 type ninjaWriter struct {
-	writer io.StringWriter
+	writer StringWriterWriter
 
 	justDidBlankLine bool // true if the last operation was a BlankLine
 }
 
-func newNinjaWriter(writer io.StringWriter) *ninjaWriter {
+func newNinjaWriter(writer StringWriterWriter) *ninjaWriter {
 	return &ninjaWriter{
 		writer: writer,
 	}
@@ -115,6 +115,8 @@ func (n *ninjaWriter) Rule(name string) error {
 
 func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 	explicitDeps, implicitDeps, orderOnlyDeps, validations []*ninjaString,
+	outputStrings, implicitOutStrings, explicitDepStrings,
+	implicitDepStrings, orderOnlyDepStrings, validationStrings []string,
 	pkgNames map[*packageContext]string) error {
 
 	n.justDidBlankLine = false
@@ -136,14 +138,22 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 
 	wrapper.WriteString("build")
 
+	for _, output := range outputStrings {
+		wrapper.Space()
+		outputEscaper.WriteString(wrapper, output)
+	}
 	for _, output := range outputs {
 		wrapper.Space()
 		output.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
 	}
 
-	if len(implicitOuts) > 0 {
+	if len(implicitOuts) > 0 || len(implicitOutStrings) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
+		for _, out := range implicitOutStrings {
+			wrapper.Space()
+			outputEscaper.WriteString(wrapper, out)
+		}
 		for _, out := range implicitOuts {
 			wrapper.Space()
 			out.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
@@ -154,32 +164,48 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 
 	wrapper.WriteStringWithSpace(rule)
 
+	for _, dep := range explicitDepStrings {
+		wrapper.Space()
+		inputEscaper.WriteString(wrapper, dep)
+	}
 	for _, dep := range explicitDeps {
 		wrapper.Space()
 		dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 	}
 
-	if len(implicitDeps) > 0 {
+	if len(implicitDeps) > 0 || len(implicitDepStrings) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
+		for _, dep := range implicitDepStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range implicitDeps {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
-	if len(orderOnlyDeps) > 0 {
+	if len(orderOnlyDeps) > 0 || len(orderOnlyDepStrings) > 0 {
 		wrapper.WriteStringWithSpace("||")
 
+		for _, dep := range orderOnlyDepStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range orderOnlyDeps {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
-	if len(validations) > 0 {
+	if len(validations) > 0 || len(validationStrings) > 0 {
 		wrapper.WriteStringWithSpace("|@")
 
+		for _, dep := range validationStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range validations {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
@@ -235,7 +261,7 @@ func (n *ninjaWriter) ScopedAssign(name, value string) error {
 	return nil
 }
 
-func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets ...*ninjaString) error {
+func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets []*ninjaString, targetStrings []string) error {
 	n.justDidBlankLine = false
 
 	const lineWrapLen = len(" $")
@@ -248,6 +274,10 @@ func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets ...*n
 
 	wrapper.WriteString("default")
 
+	for _, target := range targetStrings {
+		wrapper.Space()
+		outputEscaper.WriteString(wrapper, target)
+	}
 	for _, target := range targets {
 		wrapper.Space()
 		target.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
@@ -347,6 +377,7 @@ func (n *ninjaWriterWithWrap) WriteString(s string) (written int, noError error)
 		n.lineLen = indentWidth*2 + n.pendingLen
 		s = strings.TrimLeftFunc(s, unicode.IsSpace)
 		n.pending = append(n.pending, s)
+		n.lineLen += len(s)
 		n.writePending()
 
 		n.space = false
@@ -361,6 +392,11 @@ func (n *ninjaWriterWithWrap) WriteString(s string) (written int, noError error)
 	return
 }
 
+func (n *ninjaWriterWithWrap) Write(p []byte) (written int, noError error) {
+	// Write is rarely used, implement it via WriteString.
+	return n.WriteString(string(p))
+}
+
 // Space inserts a space that is also a possible wrapping point into the string.
 func (n *ninjaWriterWithWrap) Space() {
 	if n.err != nil {

ninja_writer_test.go

@@ -50,12 +50,15 @@ var ninjaWriterTestCases = []struct {
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Build("foo comment", "foo", testNinjaStrings("o1", "o2"),
+			ck(w.Build("foo comment", "foo", testNinjaStrings("o3", "o4"),
-				testNinjaStrings("io1", "io2"), testNinjaStrings("e1", "e2"),
+				testNinjaStrings("io3", "io4"), testNinjaStrings("e3", "e4"),
-				testNinjaStrings("i1", "i2"), testNinjaStrings("oo1", "oo2"),
+				testNinjaStrings("i3", "i4"), testNinjaStrings("oo3", "oo4"),
-				testNinjaStrings("v1", "v2"), nil))
+				testNinjaStrings("v3", "v4"), []string{"o1", "o2"},
+				[]string{"io1", "io2"}, []string{"e1", "e2"},
+				[]string{"i1", "i2"}, []string{"oo1", "oo2"},
+				[]string{"v1", "v2"}, nil))
 		},
-		output: "# foo comment\nbuild o1 o2 | io1 io2: foo e1 e2 | i1 i2 || oo1 oo2 |@ v1 v2\n",
+		output: "# foo comment\nbuild o1 o2 o3 o4 | io1 io2 io3 io4: foo e1 e2 e3 e4 | i1 i2 i3 i4 || oo1 oo2 $\n        oo3 oo4 |@ v1 v2 v3 v4\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
@@ -63,15 +66,15 @@ var ninjaWriterTestCases = []struct {
 				testNinjaStrings(strings.Repeat("o", lineWidth)),
 				nil,
 				testNinjaStrings(strings.Repeat("i", lineWidth)),
-				nil, nil, nil, nil))
+				nil, nil, nil, nil, nil, nil, nil, nil, nil, nil))
 		},
-		output: "# foo comment\nbuild $\n        " + strings.Repeat("o", lineWidth) + ": foo $\n        " + strings.Repeat("i", lineWidth) + "\n",
+		output: "# foo comment\nbuild $\n        " + strings.Repeat("o", lineWidth) + ": $\n        foo $\n        " + strings.Repeat("i", lineWidth) + "\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Default(nil, testNinjaStrings("foo")...))
+			ck(w.Default(nil, testNinjaStrings("foo"), []string{"bar"}))
 		},
-		output: "default foo\n",
+		output: "default bar foo\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
@@ -108,7 +111,8 @@ var ninjaWriterTestCases = []struct {
 			ck(w.ScopedAssign("pool", "p"))
 			ck(w.BlankLine())
 			ck(w.Build("r comment", "r", testNinjaStrings("foo.o"),
-				nil, testNinjaStrings("foo.in"), nil, nil, nil, nil))
+				nil, testNinjaStrings("foo.in"), nil, nil, nil, nil,
+				nil, nil, nil, nil, nil, nil))
 			ck(w.ScopedAssign("_arg", "arg value"))
 		},
 		output: `pool p