diff --git a/context.go b/context.go index dac1ed2..75ea2d6 100644 --- a/context.go +++ b/context.go @@ -103,15 +103,15 @@ type Context struct { // set during PrepareBuildActions pkgNames map[*packageContext]string liveGlobals *liveTracker - globalVariables map[Variable]ninjaString + globalVariables map[Variable]*ninjaString globalPools map[Pool]*poolDef globalRules map[Rule]*ruleDef // set during PrepareBuildActions - outDir ninjaString // The builddir special Ninja variable - requiredNinjaMajor int // For the ninja_required_version variable - requiredNinjaMinor int // For the ninja_required_version variable - requiredNinjaMicro int // For the ninja_required_version variable + outDir *ninjaString // The builddir special Ninja variable + requiredNinjaMajor int // For the ninja_required_version variable + requiredNinjaMinor int // For the ninja_required_version variable + requiredNinjaMicro int // For the ninja_required_version variable subninjas []string @@ -2814,7 +2814,7 @@ func jsonModuleWithActionsFromModuleInfo(m *moduleInfo) *JsonModule { // Gets a list of strings from the given list of ninjaStrings by invoking ninjaString.Value with // nil pkgNames on each of the input ninjaStrings. -func getNinjaStringsWithNilPkgNames(nStrs []ninjaString) []string { +func getNinjaStringsWithNilPkgNames(nStrs []*ninjaString) []string { var strs []string for _, nstr := range nStrs { strs = append(strs, nstr.Value(nil)) @@ -3820,7 +3820,7 @@ func (c *Context) requireNinjaVersion(major, minor, micro int) { } } -func (c *Context) setOutDir(value ninjaString) { +func (c *Context) setOutDir(value *ninjaString) { if c.outDir == nil { c.outDir = value } @@ -3901,7 +3901,7 @@ func (c *Context) memoizeFullNames(liveGlobals *liveTracker, pkgNames map[*packa } func (c *Context) checkForVariableReferenceCycles( - variables map[Variable]ninjaString, pkgNames map[*packageContext]string) { + variables map[Variable]*ninjaString, pkgNames map[*packageContext]string) { visited := make(map[Variable]bool) // variables that were already checked checking := make(map[Variable]bool) // variables actively being checked @@ -4696,7 +4696,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) string { hasher := sha256.New() for _, d := range deps { if len(d.Variables()) != 0 { @@ -4727,7 +4727,7 @@ func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.U phonyCount.Add(1) m.phony = &buildDef{ Rule: Phony, - Outputs: []ninjaString{simpleNinjaString("dedup-" + key)}, + Outputs: []*ninjaString{simpleNinjaString("dedup-" + key)}, Inputs: m.first.OrderOnly, //we could also use b.OrderOnly Optional: true, } diff --git a/context_test.go b/context_test.go index 1a1fb0d..a10f681 100644 --- a/context_test.go +++ b/context_test.go @@ -1159,10 +1159,10 @@ func TestPackageIncludes(t *testing.T) { } func TestDeduplicateOrderOnlyDeps(t *testing.T) { - outputs := func(names ...string) []ninjaString { - r := make([]ninjaString, len(names)) + outputs := func(names ...string) []*ninjaString { + r := make([]*ninjaString, len(names)) for i, name := range names { - r[i] = literalNinjaString(name) + r[i] = simpleNinjaString(name) } return r } @@ -1179,7 +1179,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) { type testcase struct { modules []*moduleInfo expectedPhonys []*buildDef - conversions map[string][]ninjaString + conversions map[string][]*ninjaString } testCases := []testcase{{ modules: []*moduleInfo{ @@ -1189,7 +1189,7 @@ 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][]*ninjaString{ "A": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"), "B": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"), }, @@ -1205,7 +1205,7 @@ 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][]*ninjaString{ "A": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"), "B": outputs("b"), "C": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"), @@ -1220,7 +1220,7 @@ 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][]*ninjaString{ "A": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"), "B": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"), "C": outputs("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"), diff --git a/live_tracker.go b/live_tracker.go index 9d4f599..e2c4057 100644 --- a/live_tracker.go +++ b/live_tracker.go @@ -25,7 +25,7 @@ type liveTracker struct { config interface{} // Used to evaluate variable, rule, and pool values. ctx *Context // Used to evaluate globs - variables map[Variable]ninjaString + variables map[Variable]*ninjaString pools map[Pool]*poolDef rules map[Rule]*ruleDef } @@ -34,7 +34,7 @@ func newLiveTracker(ctx *Context, config interface{}) *liveTracker { return &liveTracker{ ctx: ctx, config: config, - variables: make(map[Variable]ninjaString), + variables: make(map[Variable]*ninjaString), pools: make(map[Pool]*poolDef), rules: make(map[Rule]*ruleDef), } @@ -197,13 +197,13 @@ func (l *liveTracker) innerAddVariable(v Variable) error { return nil } -func (l *liveTracker) addNinjaStringListDeps(list []ninjaString) error { +func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error { l.Lock() defer l.Unlock() return l.innerAddNinjaStringListDeps(list) } -func (l *liveTracker) innerAddNinjaStringListDeps(list []ninjaString) error { +func (l *liveTracker) innerAddNinjaStringListDeps(list []*ninjaString) error { for _, str := range list { err := l.innerAddNinjaStringDeps(str) if err != nil { @@ -213,13 +213,13 @@ func (l *liveTracker) innerAddNinjaStringListDeps(list []ninjaString) error { return nil } -func (l *liveTracker) addNinjaStringDeps(str ninjaString) error { +func (l *liveTracker) addNinjaStringDeps(str *ninjaString) error { l.Lock() defer l.Unlock() return l.innerAddNinjaStringDeps(str) } -func (l *liveTracker) innerAddNinjaStringDeps(str ninjaString) error { +func (l *liveTracker) innerAddNinjaStringDeps(str *ninjaString) error { for _, v := range str.Variables() { err := l.innerAddVariable(v) if err != nil { diff --git a/ninja_defs.go b/ninja_defs.go index 69233c2..4915fdf 100644 --- a/ninja_defs.go +++ b/ninja_defs.go @@ -131,11 +131,11 @@ func (p *poolDef) WriteTo(nw *ninjaWriter, name string) error { // A ruleDef describes a rule definition. It does not include the name of the // rule. type ruleDef struct { - CommandDeps []ninjaString - CommandOrderOnly []ninjaString + CommandDeps []*ninjaString + CommandOrderOnly []*ninjaString Comment string Pool Pool - Variables map[string]ninjaString + Variables map[string]*ninjaString } func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, @@ -144,7 +144,7 @@ func parseRuleParams(scope scope, params *RuleParams) (*ruleDef, r := &ruleDef{ Comment: params.Comment, Pool: params.Pool, - Variables: make(map[string]ninjaString), + Variables: make(map[string]*ninjaString), } if params.Command == "" { @@ -264,14 +264,14 @@ type buildDef struct { Comment string Rule Rule RuleDef *ruleDef - Outputs []ninjaString - ImplicitOutputs []ninjaString - Inputs []ninjaString - Implicits []ninjaString - OrderOnly []ninjaString - Validations []ninjaString - Args map[Variable]ninjaString - Variables map[string]ninjaString + Outputs []*ninjaString + ImplicitOutputs []*ninjaString + Inputs []*ninjaString + Implicits []*ninjaString + OrderOnly []*ninjaString + Validations []*ninjaString + Args map[Variable]*ninjaString + Variables map[string]*ninjaString Optional bool } @@ -286,9 +286,9 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef, Rule: rule, } - setVariable := func(name string, value ninjaString) { + setVariable := func(name string, value *ninjaString) { if b.Variables == nil { - b.Variables = make(map[string]ninjaString) + b.Variables = make(map[string]*ninjaString) } b.Variables[name] = value } @@ -363,7 +363,7 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef, argNameScope := rule.scope() if len(params.Args) > 0 { - b.Args = make(map[Variable]ninjaString) + b.Args = make(map[Variable]*ninjaString) for name, value := range params.Args { if !rule.isArg(name) { return nil, fmt.Errorf("unknown argument %q", name) @@ -444,7 +444,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) return nw.BlankLine() } -func writeVariables(nw *ninjaWriter, variables map[string]ninjaString, +func writeVariables(nw *ninjaWriter, variables map[string]*ninjaString, pkgNames map[*packageContext]string) error { var keys []string for k := range variables { diff --git a/ninja_strings.go b/ninja_strings.go index 9e83a4d..c3b070e 100644 --- a/ninja_strings.go +++ b/ninja_strings.go @@ -35,19 +35,28 @@ var ( ":", "$:") ) -type ninjaString interface { - Value(pkgNames map[*packageContext]string) string - ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string, escaper *strings.Replacer) - Eval(variables map[Variable]ninjaString) (string, error) - Variables() []Variable +// ninjaString contains the parsed result of a string that can contain references to variables (e.g. $cflags) that will +// be propagated to the build.ninja file. For literal strings with no variable references, the variables field will be +// nil. For strings with variable references str contains the original, unparsed string, and variables contains a +// pointer to a list of references, each with a span of bytes they should replace and a Variable interface. +type ninjaString struct { + str string + variables *[]variableReference } -type varNinjaString struct { - strings []string - variables []Variable -} +// variableReference contains information about a single reference to a variable (e.g. $cflags) inside a parsed +// ninjaString. start and end are int32 to reduce memory usage. A nil variable is a special case of an inserted '$' +// at the beginning of the string to handle leading whitespace that must not be stripped by ninja. +type variableReference struct { + // start is the offset of the '$' character from the beginning of the unparsed string. + start int32 -type literalNinjaString string + // end is the offset of the character _after_ the final character of the variable name (or '}' if using the + //'${}' syntax) + end int32 + + variable Variable +} type scope interface { LookupVariable(name string) (Variable, error) @@ -55,36 +64,24 @@ type scope interface { IsPoolVisible(pool Pool) bool } -func simpleNinjaString(str string) ninjaString { - return literalNinjaString(str) +func simpleNinjaString(str string) *ninjaString { + return &ninjaString{str: str} } type parseState struct { - scope scope - str string - pendingStr string - stringStart int - varStart int - result *varNinjaString + scope scope + str string + varStart int + varNameStart int + result *ninjaString } -func (ps *parseState) pushVariable(v Variable) { - if len(ps.result.variables) == len(ps.result.strings) { - // Last push was a variable, we need a blank string separator - ps.result.strings = append(ps.result.strings, "") +func (ps *parseState) pushVariable(start, end int, v Variable) { + if ps.result.variables == nil { + ps.result.variables = &[]variableReference{{start: int32(start), end: int32(end), variable: v}} + } else { + *ps.result.variables = append(*ps.result.variables, variableReference{start: int32(start), end: int32(end), variable: v}) } - if ps.pendingStr != "" { - panic("oops, pushed variable with pending string") - } - ps.result.variables = append(ps.result.variables, v) -} - -func (ps *parseState) pushString(s string) { - if len(ps.result.strings) != len(ps.result.variables) { - panic("oops, pushed string after string") - } - ps.result.strings = append(ps.result.strings, ps.pendingStr+s) - ps.pendingStr = "" } type stateFunc func(*parseState, int, rune) (stateFunc, error) @@ -92,18 +89,19 @@ type stateFunc func(*parseState, int, rune) (stateFunc, error) // parseNinjaString parses an unescaped ninja string (i.e. all $ // occurrences are expected to be variables or $$) and returns a list of the // variable names that the string references. -func parseNinjaString(scope scope, str string) (ninjaString, error) { - // naively pre-allocate slices by counting $ signs +func parseNinjaString(scope scope, str string) (*ninjaString, error) { + // naively pre-allocate slice by counting $ signs n := strings.Count(str, "$") if n == 0 { - if strings.HasPrefix(str, " ") { + if len(str) > 0 && str[0] == ' ' { str = "$" + str } - return literalNinjaString(str), nil + return simpleNinjaString(str), nil } - result := &varNinjaString{ - strings: make([]string, 0, n+1), - variables: make([]Variable, 0, n), + variableReferences := make([]variableReference, 0, n) + result := &ninjaString{ + str: str, + variables: &variableReferences, } parseState := &parseState{ @@ -127,12 +125,18 @@ func parseNinjaString(scope scope, str string) (ninjaString, error) { return nil, err } + // All the '$' characters counted initially could have been "$$" escapes, leaving no + // variable references. Deallocate the variables slice if so. + if len(*result.variables) == 0 { + result.variables = nil + } + return result, nil } func parseFirstRuneState(state *parseState, i int, r rune) (stateFunc, error) { if r == ' ' { - state.pendingStr += "$" + state.pushVariable(0, 1, nil) } return parseStringState(state, i, r) } @@ -140,11 +144,10 @@ func parseFirstRuneState(state *parseState, i int, r rune) (stateFunc, error) { func parseStringState(state *parseState, i int, r rune) (stateFunc, error) { switch { case r == '$': - state.varStart = i + 1 + state.varStart = i return parseDollarStartState, nil case r == eof: - state.pushString(state.str[state.stringStart:i]) return nil, nil default: @@ -156,21 +159,17 @@ func parseDollarStartState(state *parseState, i int, r rune) (stateFunc, error) switch { case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '_', r == '-': - // The beginning of a of the variable name. Output the string and - // keep going. - state.pushString(state.str[state.stringStart : i-1]) + // The beginning of a of the variable name. + state.varNameStart = i return parseDollarState, nil case r == '$': - // Just a "$$". Go back to parseStringState without changing - // state.stringStart. + // Just a "$$". Go back to parseStringState. return parseStringState, nil case r == '{': - // This is a bracketted variable name (e.g. "${blah.blah}"). Output - // the string and keep going. - state.pushString(state.str[state.stringStart : i-1]) - state.varStart = i + 1 + // This is a bracketted variable name (e.g. "${blah.blah}"). + state.varNameStart = i + 1 return parseBracketsState, nil case r == eof: @@ -190,45 +189,26 @@ func parseDollarState(state *parseState, i int, r rune) (stateFunc, error) { r >= '0' && r <= '9', r == '_', r == '-': // A part of the variable name. Keep going. return parseDollarState, nil + } + // The variable name has ended, output what we have. + v, err := state.scope.LookupVariable(state.str[state.varNameStart:i]) + if err != nil { + return nil, err + } + + state.pushVariable(state.varStart, i, v) + + switch { case r == '$': - // A dollar after the variable name (e.g. "$blah$"). Output the - // variable we have and start a new one. - v, err := state.scope.LookupVariable(state.str[state.varStart:i]) - if err != nil { - return nil, err - } - - state.pushVariable(v) - state.varStart = i + 1 - state.stringStart = i - + // A dollar after the variable name (e.g. "$blah$"). Start a new one. + state.varStart = i return parseDollarStartState, nil case r == eof: - // This is the end of the variable name. - v, err := state.scope.LookupVariable(state.str[state.varStart:i]) - if err != nil { - return nil, err - } - - state.pushVariable(v) - - // We always end with a string, even if it's an empty one. - state.pushString("") - return nil, nil default: - // We've just gone past the end of the variable name, so record what - // we have. - v, err := state.scope.LookupVariable(state.str[state.varStart:i]) - if err != nil { - return nil, err - } - - state.pushVariable(v) - state.stringStart = i return parseStringState, nil } } @@ -241,20 +221,19 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) { return parseBracketsState, nil case r == '}': - if state.varStart == i { + if state.varNameStart == i { // The brackets were immediately closed. That's no good. return nil, fmt.Errorf("empty variable name at byte offset %d", i) } // This is the end of the variable name. - v, err := state.scope.LookupVariable(state.str[state.varStart:i]) + v, err := state.scope.LookupVariable(state.str[state.varNameStart:i]) if err != nil { return nil, err } - state.pushVariable(v) - state.stringStart = i + 1 + state.pushVariable(state.varStart, i+1, v) return parseStringState, nil case r == eof: @@ -267,13 +246,13 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) { } } -func parseNinjaStrings(scope scope, strs []string) ([]ninjaString, +func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString, error) { if len(strs) == 0 { return nil, nil } - result := make([]ninjaString, len(strs)) + result := make([]*ninjaString, len(strs)) for i, str := range strs { ninjaStr, err := parseNinjaString(scope, str) if err != nil { @@ -284,62 +263,78 @@ func parseNinjaStrings(scope scope, strs []string) ([]ninjaString, return result, nil } -func (n varNinjaString) Value(pkgNames map[*packageContext]string) string { - if len(n.strings) == 1 { - return defaultEscaper.Replace(n.strings[0]) +func (n *ninjaString) Value(pkgNames map[*packageContext]string) string { + if n.variables == nil || len(*n.variables) == 0 { + return defaultEscaper.Replace(n.str) } str := &strings.Builder{} n.ValueWithEscaper(str, pkgNames, defaultEscaper) return str.String() } -func (n varNinjaString) ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string, +func (n *ninjaString) ValueWithEscaper(w io.StringWriter, pkgNames map[*packageContext]string, escaper *strings.Replacer) { - w.WriteString(escaper.Replace(n.strings[0])) - for i, v := range n.variables { - w.WriteString("${") - w.WriteString(v.fullName(pkgNames)) - w.WriteString("}") - w.WriteString(escaper.Replace(n.strings[i+1])) + if n.variables == nil || len(*n.variables) == 0 { + w.WriteString(escaper.Replace(n.str)) + return } -} -func (n varNinjaString) Eval(variables map[Variable]ninjaString) (string, error) { - str := n.strings[0] - for i, v := range n.variables { - variable, ok := variables[v] - if !ok { - return "", fmt.Errorf("no such global variable: %s", v) + i := 0 + for _, v := range *n.variables { + w.WriteString(escaper.Replace(n.str[i:v.start])) + if v.variable == nil { + w.WriteString("$ ") + } else { + w.WriteString("${") + w.WriteString(v.variable.fullName(pkgNames)) + w.WriteString("}") } - value, err := variable.Eval(variables) - if err != nil { - return "", err - } - str += value + n.strings[i+1] + i = int(v.end) } - return str, nil + w.WriteString(escaper.Replace(n.str[i:len(n.str)])) } -func (n varNinjaString) Variables() []Variable { - return n.variables +func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) { + if n.variables == nil || len(*n.variables) == 0 { + return n.str, nil + } + + w := &strings.Builder{} + i := 0 + for _, v := range *n.variables { + w.WriteString(n.str[i:v.start]) + if v.variable == nil { + w.WriteString(" ") + } else { + variable, ok := variables[v.variable] + if !ok { + return "", fmt.Errorf("no such global variable: %s", v.variable) + } + value, err := variable.Eval(variables) + if err != nil { + return "", err + } + w.WriteString(value) + } + i = int(v.end) + } + w.WriteString(n.str[i:len(n.str)]) + return w.String(), nil } -func (l literalNinjaString) Value(_ map[*packageContext]string) string { - return defaultEscaper.Replace(string(l)) -} +func (n *ninjaString) Variables() []Variable { + if n.variables == nil || len(*n.variables) == 0 { + return nil + } -func (l literalNinjaString) ValueWithEscaper(w io.StringWriter, _ map[*packageContext]string, - escaper *strings.Replacer) { - w.WriteString(escaper.Replace(string(l))) -} - -func (l literalNinjaString) Eval(variables map[Variable]ninjaString) (string, error) { - return string(l), nil -} - -func (l literalNinjaString) Variables() []Variable { - return nil + variables := make([]Variable, 0, len(*n.variables)) + for _, v := range *n.variables { + if v.variable != nil { + variables = append(variables, v.variable) + } + } + return variables } func validateNinjaName(name string) error { diff --git a/ninja_strings_test.go b/ninja_strings_test.go index f400074..2fa4937 100644 --- a/ninja_strings_test.go +++ b/ninja_strings_test.go @@ -21,143 +21,176 @@ import ( "testing" ) -var ninjaParseTestCases = []struct { - input string - vars []string - strs []string - literal bool - err string -}{ - { - input: "abc def $ghi jkl", - vars: []string{"ghi"}, - strs: []string{"abc def ", " jkl"}, - }, - { - input: "abc def $ghi$jkl", - vars: []string{"ghi", "jkl"}, - strs: []string{"abc def ", "", ""}, - }, - { - input: "foo $012_-345xyz_! bar", - vars: []string{"012_-345xyz_"}, - strs: []string{"foo ", "! bar"}, - }, - { - input: "foo ${012_-345xyz_} bar", - vars: []string{"012_-345xyz_"}, - strs: []string{"foo ", " bar"}, - }, - { - input: "foo ${012_-345xyz_} bar", - vars: []string{"012_-345xyz_"}, - strs: []string{"foo ", " bar"}, - }, - { - input: "foo $$ bar", - vars: nil, - strs: []string{"foo $$ bar"}, - // this is technically a literal, but not recognized as such due to the $$ - }, - { - input: "$foo${bar}", - vars: []string{"foo", "bar"}, - strs: []string{"", "", ""}, - }, - { - input: "$foo$$", - vars: []string{"foo"}, - strs: []string{"", "$$"}, - }, - { - input: "foo bar", - vars: nil, - strs: []string{"foo bar"}, - literal: true, - }, - { - input: " foo ", - vars: nil, - strs: []string{"$ foo "}, - literal: true, - }, - { - input: " $foo ", - vars: []string{"foo"}, - strs: []string{"$ ", " "}, - }, { - input: "foo $ bar", - err: `error parsing ninja string "foo $ bar": invalid character after '$' at byte offset 5`, - }, - { - input: "foo $", - err: "unexpected end of string after '$'", - }, - { - input: "foo ${} bar", - err: `error parsing ninja string "foo ${} bar": empty variable name at byte offset 6`, - }, - { - input: "foo ${abc!} bar", - err: `error parsing ninja string "foo ${abc!} bar": invalid character in variable name at byte offset 9`, - }, - { - input: "foo ${abc", - err: "unexpected end of string in variable name", - }, +type testVariableRef struct { + start, end int + name string } func TestParseNinjaString(t *testing.T) { - for _, testCase := range ninjaParseTestCases { - scope := newLocalScope(nil, "namespace") - expectedVars := []Variable{} - for _, varName := range testCase.vars { - v, err := scope.LookupVariable(varName) - if err != nil { - v, err = scope.AddLocalVariable(varName, "") + testCases := []struct { + input string + vars []string + value string + eval string + err string + }{ + { + input: "abc def $ghi jkl", + vars: []string{"ghi"}, + value: "abc def ${namespace.ghi} jkl", + eval: "abc def GHI jkl", + }, + { + input: "abc def $ghi$jkl", + vars: []string{"ghi", "jkl"}, + value: "abc def ${namespace.ghi}${namespace.jkl}", + eval: "abc def GHIJKL", + }, + { + input: "foo $012_-345xyz_! bar", + vars: []string{"012_-345xyz_"}, + value: "foo ${namespace.012_-345xyz_}! bar", + eval: "foo 012_-345XYZ_! bar", + }, + { + input: "foo ${012_-345xyz_} bar", + vars: []string{"012_-345xyz_"}, + value: "foo ${namespace.012_-345xyz_} bar", + eval: "foo 012_-345XYZ_ bar", + }, + { + input: "foo ${012_-345xyz_} bar", + vars: []string{"012_-345xyz_"}, + value: "foo ${namespace.012_-345xyz_} bar", + eval: "foo 012_-345XYZ_ bar", + }, + { + input: "foo $$ bar", + vars: nil, + value: "foo $$ bar", + eval: "foo $$ bar", + }, + { + input: "$foo${bar}", + vars: []string{"foo", "bar"}, + value: "${namespace.foo}${namespace.bar}", + eval: "FOOBAR", + }, + { + input: "$foo$$", + vars: []string{"foo"}, + value: "${namespace.foo}$$", + eval: "FOO$$", + }, + { + input: "foo bar", + vars: nil, + value: "foo bar", + eval: "foo bar", + }, + { + input: " foo ", + vars: nil, + value: "$ foo ", + eval: "$ foo ", + }, + { + input: "\tfoo ", + vars: nil, + value: "\tfoo ", + eval: "\tfoo ", + }, + { + input: "\nfoo ", + vars: nil, + value: "$\nfoo ", + eval: "\nfoo ", + }, + { + input: " $foo ", + vars: []string{"foo"}, + value: "$ ${namespace.foo} ", + eval: " FOO ", + }, + { + input: "\t$foo ", + vars: []string{"foo"}, + value: "\t${namespace.foo} ", + eval: "\tFOO ", + }, + { + input: "\n$foo ", + vars: []string{"foo"}, + value: "$\n${namespace.foo} ", + eval: "\nFOO ", + }, + { + input: "foo $ bar", + err: `error parsing ninja string "foo $ bar": invalid character after '$' at byte offset 5`, + }, + { + input: "foo $", + err: "unexpected end of string after '$'", + }, + { + input: "foo ${} bar", + err: `error parsing ninja string "foo ${} bar": empty variable name at byte offset 6`, + }, + { + input: "foo ${abc!} bar", + err: `error parsing ninja string "foo ${abc!} bar": invalid character in variable name at byte offset 9`, + }, + { + input: "foo ${abc", + err: "unexpected end of string in variable name", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.input, func(t *testing.T) { + scope := newLocalScope(nil, "namespace.") + variablesMap := map[Variable]*ninjaString{} + for _, varName := range testCase.vars { + _, err := scope.LookupVariable(varName) if err != nil { - t.Fatalf("error creating scope: %s", err) + v, err := scope.AddLocalVariable(varName, strings.ToUpper(varName)) + if err != nil { + t.Fatalf("error creating scope: %s", err) + } + variablesMap[v] = simpleNinjaString(strings.ToUpper(varName)) } } - expectedVars = append(expectedVars, v) - } - var expected ninjaString - if len(testCase.strs) > 0 { - if testCase.literal { - expected = literalNinjaString(testCase.strs[0]) - } else { - expected = &varNinjaString{ - strings: testCase.strs, - variables: expectedVars, + output, err := parseNinjaString(scope, testCase.input) + if err == nil { + if g, w := output.Value(nil), testCase.value; g != w { + t.Errorf("incorrect Value output, want %q, got %q", w, g) + } + + eval, err := output.Eval(variablesMap) + if err != nil { + t.Errorf("unexpected error in Eval: %s", err) + } + if g, w := eval, testCase.eval; g != w { + t.Errorf("incorrect Eval output, want %q, got %q", w, g) } } - } - - output, err := parseNinjaString(scope, testCase.input) - if err == nil { - if !reflect.DeepEqual(output, expected) { - t.Errorf("incorrect ninja string:") + var errStr string + if err != nil { + errStr = err.Error() + } + if err != nil && err.Error() != testCase.err { + t.Errorf("unexpected error:") t.Errorf(" input: %q", testCase.input) - t.Errorf(" expected: %#v", expected) - t.Errorf(" got: %#v", output) + t.Errorf(" expected: %q", testCase.err) + t.Errorf(" got: %q", errStr) } - } - var errStr string - if err != nil { - errStr = err.Error() - } - if err != nil && err.Error() != testCase.err { - t.Errorf("unexpected error:") - t.Errorf(" input: %q", testCase.input) - t.Errorf(" expected: %q", testCase.err) - t.Errorf(" got: %q", errStr) - } + }) } } func TestParseNinjaStringWithImportedVar(t *testing.T) { - ImpVar := &staticVariable{name_: "ImpVar"} + ImpVar := &staticVariable{name_: "ImpVar", fullName_: "g.impPkg.ImpVar"} impScope := newScope(nil) impScope.AddVariable(ImpVar) scope := newScope(nil) @@ -169,13 +202,59 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) { t.Fatalf("unexpected error: %s", err) } - expect := []Variable{ImpVar} - if !reflect.DeepEqual(output.(*varNinjaString).variables, expect) { + expect := []variableReference{{8, 24, ImpVar}} + if !reflect.DeepEqual(*output.variables, expect) { t.Errorf("incorrect output:") t.Errorf(" input: %q", input) t.Errorf(" expected: %#v", expect) - t.Errorf(" got: %#v", output) + t.Errorf(" got: %#v", *output.variables) } + + if g, w := output.Value(nil), "abc def ${g.impPkg.ImpVar} ghi"; g != w { + t.Errorf("incorrect Value output, want %q got %q", w, g) + } +} + +func Benchmark_parseNinjaString(b *testing.B) { + b.Run("constant", func(b *testing.B) { + for _, l := range []int{1, 10, 100, 1000} { + b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() + for n := 0; n < b.N; n++ { + _ = simpleNinjaString(strings.Repeat("a", l)) + } + }) + } + }) + b.Run("variable", func(b *testing.B) { + for _, l := range []int{1, 10, 100, 1000} { + scope := newLocalScope(nil, "") + scope.AddLocalVariable("a", strings.Repeat("b", l/3)) + b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() + for n := 0; n < b.N; n++ { + _, _ = parseNinjaString(scope, strings.Repeat("a", l/3)+"${a}"+strings.Repeat("a", l/3)) + } + }) + } + }) + b.Run("variables", func(b *testing.B) { + for _, l := range []int{1, 2, 3, 4, 5, 10, 100, 1000} { + scope := newLocalScope(nil, "") + str := strings.Repeat("a", 10) + for i := 0; i < l; i++ { + scope.AddLocalVariable("a"+strconv.Itoa(i), strings.Repeat("b", 10)) + str += "${a" + strconv.Itoa(i) + "}" + } + b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() + for n := 0; n < b.N; n++ { + _, _ = parseNinjaString(scope, str) + } + }) + } + }) + } func BenchmarkNinjaString_Value(b *testing.B) { @@ -183,6 +262,7 @@ func BenchmarkNinjaString_Value(b *testing.B) { for _, l := range []int{1, 10, 100, 1000} { ns := simpleNinjaString(strings.Repeat("a", l)) b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() for n := 0; n < b.N; n++ { ns.Value(nil) } @@ -195,6 +275,7 @@ func BenchmarkNinjaString_Value(b *testing.B) { scope.AddLocalVariable("a", strings.Repeat("b", l/3)) ns, _ := parseNinjaString(scope, strings.Repeat("a", l/3)+"${a}"+strings.Repeat("a", l/3)) b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() for n := 0; n < b.N; n++ { ns.Value(nil) } @@ -211,6 +292,7 @@ func BenchmarkNinjaString_Value(b *testing.B) { } ns, _ := parseNinjaString(scope, str) b.Run(strconv.Itoa(l), func(b *testing.B) { + b.ReportAllocs() for n := 0; n < b.N; n++ { ns.Value(nil) } diff --git a/ninja_writer.go b/ninja_writer.go index f9951b4..fbbfb9e 100644 --- a/ninja_writer.go +++ b/ninja_writer.go @@ -114,7 +114,7 @@ func (n *ninjaWriter) Rule(name string) error { } func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts, - explicitDeps, implicitDeps, orderOnlyDeps, validations []ninjaString, + explicitDeps, implicitDeps, orderOnlyDeps, validations []*ninjaString, pkgNames map[*packageContext]string) error { n.justDidBlankLine = false @@ -235,7 +235,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) error { n.justDidBlankLine = false const lineWrapLen = len(" $") diff --git a/ninja_writer_test.go b/ninja_writer_test.go index 82eeee5..c70d5df 100644 --- a/ninja_writer_test.go +++ b/ninja_writer_test.go @@ -139,7 +139,7 @@ func TestNinjaWriter(t *testing.T) { } } -func testNinjaStrings(s ...string) []ninjaString { +func testNinjaStrings(s ...string) []*ninjaString { ret, _ := parseNinjaStrings(nil, s) return ret } diff --git a/package_ctx.go b/package_ctx.go index f8a7cc4..4b48337 100644 --- a/package_ctx.go +++ b/package_ctx.go @@ -304,7 +304,7 @@ func (v *staticVariable) memoizeFullName(pkgNames map[*packageContext]string) { v.fullName_ = v.fullName(pkgNames) } -func (v *staticVariable) value(VariableFuncContext, interface{}) (ninjaString, error) { +func (v *staticVariable) value(VariableFuncContext, interface{}) (*ninjaString, error) { ninjaStr, err := parseNinjaString(v.pctx.scope, v.value_) if err != nil { err = fmt.Errorf("error parsing variable %s value: %s", v, err) @@ -440,7 +440,7 @@ func (v *variableFunc) memoizeFullName(pkgNames map[*packageContext]string) { v.fullName_ = v.fullName(pkgNames) } -func (v *variableFunc) value(ctx VariableFuncContext, config interface{}) (ninjaString, error) { +func (v *variableFunc) value(ctx VariableFuncContext, config interface{}) (*ninjaString, error) { value, err := v.value_(ctx, config) if err != nil { return nil, err @@ -504,7 +504,7 @@ func (v *argVariable) memoizeFullName(pkgNames map[*packageContext]string) { // Nothing to do, full name is known at initialization. } -func (v *argVariable) value(ctx VariableFuncContext, config interface{}) (ninjaString, error) { +func (v *argVariable) value(ctx VariableFuncContext, config interface{}) (*ninjaString, error) { return nil, errVariableIsArg } diff --git a/scope.go b/scope.go index 9585559..0ca3464 100644 --- a/scope.go +++ b/scope.go @@ -29,7 +29,7 @@ type Variable interface { name() string // "foo" fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo" memoizeFullName(pkgNames map[*packageContext]string) // precompute fullName if desired - value(ctx VariableFuncContext, config interface{}) (ninjaString, error) + value(ctx VariableFuncContext, config interface{}) (*ninjaString, error) String() string } @@ -354,7 +354,7 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams, type localVariable struct { fullName_ string name_ string - value_ ninjaString + value_ *ninjaString } func (l *localVariable) packageContext() *packageContext { @@ -373,7 +373,7 @@ func (l *localVariable) memoizeFullName(pkgNames map[*packageContext]string) { // Nothing to do, full name is known at initialization. } -func (l *localVariable) value(VariableFuncContext, interface{}) (ninjaString, error) { +func (l *localVariable) value(VariableFuncContext, interface{}) (*ninjaString, error) { return l.value_, nil }