Improve makefile parser am: 08693d2bf2

am: 21b30618ae

* commit '21b30618ae3633491deee09decbca92b05ac3993':
  Improve makefile parser

Change-Id: If0d95586af952280be210028a4a849fe7c4afc9a

Android.bp

@@ -216,8 +216,8 @@ bootstrap_go_package {
     name: "androidmk-parser",
     pkgPath: "android/soong/androidmk/parser",
     srcs: [
+        "androidmk/parser/ast.go",
         "androidmk/parser/make_strings.go",
-        "androidmk/parser/makething.go",
         "androidmk/parser/parser.go",
         "androidmk/parser/scope.go",
     ],

androidmk/cmd/androidmk/androidmk.go

@@ -29,8 +29,8 @@ type bpFile struct {
 	inModule bool
 }
 
-func (f *bpFile) errorf(thing mkparser.MakeThing, s string, args ...interface{}) {
+func (f *bpFile) errorf(node mkparser.Node, s string, args ...interface{}) {
-	orig := thing.Dump()
+	orig := node.Dump()
 	s = fmt.Sprintf(s, args...)
 	c := bpparser.Comment{
 		Comment: []string{fmt.Sprintf("// ANDROIDMK TRANSLATION ERROR: %s", s)},
@@ -73,7 +73,7 @@ func main() {
 
 	p := mkparser.NewParser(os.Args[1], bytes.NewBuffer(b))
 
-	things, errs := p.Parse()
+	nodes, errs := p.Parse()
 	if len(errs) > 0 {
 		for _, err := range errs {
 			fmt.Println("ERROR: ", err)
@@ -90,33 +90,34 @@ func main() {
 	var conds []*conditional
 	var assignmentCond *conditional
 
-	for _, t := range things {
+	for _, node := range nodes {
-		file.setMkPos(t.Pos(), t.EndPos())
+		file.setMkPos(p.Unpack(node.Pos()), p.Unpack(node.End()))
 
-		if comment, ok := t.AsComment(); ok {
+		switch x := node.(type) {
+		case *mkparser.Comment:
 			file.comments = append(file.comments, bpparser.Comment{
-				Comment: []string{"//" + comment.Comment},
 				Pos:     file.bpPos,
+				Comment: []string{"//" + x.Comment},
 			})
-		} else if assignment, ok := t.AsAssignment(); ok {
+		case *mkparser.Assignment:
-			handleAssignment(file, assignment, assignmentCond)
+			handleAssignment(file, x, assignmentCond)
-		} else if directive, ok := t.AsDirective(); ok {
+		case *mkparser.Directive:
-			switch directive.Name {
+			switch x.Name {
 			case "include":
-				val := directive.Args.Value(file.scope)
+				val := x.Args.Value(file.scope)
 				switch {
 				case soongModuleTypes[val]:
-					handleModuleConditionals(file, directive, conds)
+					handleModuleConditionals(file, x, conds)
 					makeModule(file, val)
 				case val == clear_vars:
 					resetModule(file)
 				default:
-					file.errorf(directive, "unsupported include")
+					file.errorf(x, "unsupported include")
 					continue
 				}
 			case "ifeq", "ifneq", "ifdef", "ifndef":
-				args := directive.Args.Dump()
+				args := x.Args.Dump()
-				eq := directive.Name == "ifeq" || directive.Name == "ifdef"
+				eq := x.Name == "ifeq" || x.Name == "ifdef"
 				if _, ok := conditionalTranslations[args]; ok {
 					newCond := conditional{args, eq}
 					conds = append(conds, &newCond)
@@ -124,29 +125,29 @@ func main() {
 						if assignmentCond == nil {
 							assignmentCond = &newCond
 						} else {
-							file.errorf(directive, "unsupported nested conditional in module")
+							file.errorf(x, "unsupported nested conditional in module")
 						}
 					}
 				} else {
-					file.errorf(directive, "unsupported conditional")
+					file.errorf(x, "unsupported conditional")
 					conds = append(conds, nil)
 					continue
 				}
 			case "else":
 				if len(conds) == 0 {
-					file.errorf(directive, "missing if before else")
+					file.errorf(x, "missing if before else")
 					continue
 				} else if conds[len(conds)-1] == nil {
-					file.errorf(directive, "else from unsupported contitional")
+					file.errorf(x, "else from unsupported contitional")
 					continue
 				}
 				conds[len(conds)-1].eq = !conds[len(conds)-1].eq
 			case "endif":
 				if len(conds) == 0 {
-					file.errorf(directive, "missing if before endif")
+					file.errorf(x, "missing if before endif")
 					continue
 				} else if conds[len(conds)-1] == nil {
-					file.errorf(directive, "endif from unsupported contitional")
+					file.errorf(x, "endif from unsupported contitional")
 					conds = conds[:len(conds)-1]
 				} else {
 					if assignmentCond == conds[len(conds)-1] {
@@ -155,11 +156,11 @@ func main() {
 					conds = conds[:len(conds)-1]
 				}
 			default:
-				file.errorf(directive, "unsupported directive")
+				file.errorf(x, "unsupported directive")
 				continue
 			}
-		} else {
+		default:
-			file.errorf(t, "unsupported line")
+			file.errorf(x, "unsupported line")
 		}
 	}
 
@@ -175,7 +176,7 @@ func main() {
 	fmt.Print(string(out))
 }
 
-func handleAssignment(file *bpFile, assignment mkparser.Assignment, c *conditional) {
+func handleAssignment(file *bpFile, assignment *mkparser.Assignment, c *conditional) {
 	if !assignment.Name.Const() {
 		file.errorf(assignment, "unsupported non-const variable name")
 		return
@@ -239,7 +240,7 @@ func handleAssignment(file *bpFile, assignment mkparser.Assignment, c *condition
 			// This is a hack to get the LOCAL_ARM_MODE value inside
 			// of an arch: { arm: {} } block.
 			armModeAssign := assignment
-			armModeAssign.Name = mkparser.SimpleMakeString("LOCAL_ARM_MODE_HACK_arm", assignment.Name.Pos)
+			armModeAssign.Name = mkparser.SimpleMakeString("LOCAL_ARM_MODE_HACK_arm", assignment.Name.Pos())
 			handleAssignment(file, armModeAssign, c)
 		case name == "LOCAL_ADDITIONAL_DEPENDENCIES":
 			// TODO: check for only .mk files?
@@ -257,7 +258,7 @@ func handleAssignment(file *bpFile, assignment mkparser.Assignment, c *condition
 	}
 }
 
-func handleModuleConditionals(file *bpFile, directive mkparser.Directive, conds []*conditional) {
+func handleModuleConditionals(file *bpFile, directive *mkparser.Directive, conds []*conditional) {
 	for _, c := range conds {
 		if c == nil {
 			continue
@@ -270,7 +271,7 @@ func handleModuleConditionals(file *bpFile, directive mkparser.Directive, conds
 		disabledPrefix := conditionalTranslations[c.cond][!c.eq]
 
 		// Create a fake assignment with enabled = false
-		val, err := makeVariableToBlueprint(file, mkparser.SimpleMakeString("false", file.bpPos), bpparser.Bool)
+		val, err := makeVariableToBlueprint(file, mkparser.SimpleMakeString("false", mkparser.NoPos), bpparser.Bool)
 		if err == nil {
 			err = setVariable(file, false, disabledPrefix, "enabled", val, true)
 		}

androidmk/parser/ast.go

@@ -0,0 +1,110 @@
+package parser
+
+type Pos int
+
+const NoPos Pos = 0
+
+type Node interface {
+	Dump() string
+	Pos() Pos
+	End() Pos
+}
+
+type Assignment struct {
+	Target *MakeString
+	Name   *MakeString
+	Value  *MakeString
+	Type   string
+}
+
+func (x *Assignment) Dump() string {
+	target := ""
+	if x.Target != nil {
+		target = x.Target.Dump() + ": "
+	}
+	return target + x.Name.Dump() + x.Type + x.Value.Dump()
+}
+
+func (x *Assignment) Pos() Pos {
+	if x.Target != nil {
+		return x.Target.Pos()
+	}
+	return x.Name.Pos()
+}
+
+func (x *Assignment) End() Pos { return x.Value.End() }
+
+type Comment struct {
+	CommentPos Pos
+	Comment    string
+}
+
+func (x *Comment) Dump() string {
+	return "#" + x.Comment
+}
+
+func (x *Comment) Pos() Pos { return x.CommentPos }
+func (x *Comment) End() Pos { return Pos(int(x.CommentPos) + len(x.Comment)) }
+
+type Directive struct {
+	NamePos Pos
+	Name    string
+	Args    *MakeString
+	EndPos  Pos
+}
+
+func (x *Directive) Dump() string {
+	return x.Name + " " + x.Args.Dump()
+}
+
+func (x *Directive) Pos() Pos { return x.NamePos }
+func (x *Directive) End() Pos {
+	if x.EndPos != NoPos {
+		return x.EndPos
+	}
+	return x.Args.End()
+}
+
+type Rule struct {
+	Target        *MakeString
+	Prerequisites *MakeString
+	RecipePos     Pos
+	Recipe        string
+}
+
+func (x *Rule) Dump() string {
+	recipe := ""
+	if x.Recipe != "" {
+		recipe = "\n" + x.Recipe
+	}
+	return "rule:       " + x.Target.Dump() + ": " + x.Prerequisites.Dump() + recipe
+}
+
+func (x *Rule) Pos() Pos { return x.Target.Pos() }
+func (x *Rule) End() Pos { return Pos(int(x.RecipePos) + len(x.Recipe)) }
+
+type Variable struct {
+	Name *MakeString
+}
+
+func (x *Variable) Pos() Pos { return x.Name.Pos() }
+func (x *Variable) End() Pos { return x.Name.End() }
+
+func (x *Variable) Dump() string {
+	return "$(" + x.Name.Dump() + ")"
+}
+
+// Sort interface for []Node by position
+type byPosition []Node
+
+func (s byPosition) Len() int {
+	return len(s)
+}
+
+func (s byPosition) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+
+func (s byPosition) Less(i, j int) bool {
+	return s[i].Pos() < s[j].Pos()
+}

androidmk/parser/make_strings.go

@@ -2,7 +2,6 @@ package parser
 
 import (
 	"strings"
-	"text/scanner"
 	"unicode"
 )
 
@@ -18,18 +17,30 @@ import (
 // of Variables.  The raw string list is always one longer than the variable
 // list.
 type MakeString struct {
-	Pos       scanner.Position
+	StringPos Pos
 	Strings   []string
 	Variables []Variable
 }
 
-func SimpleMakeString(s string, pos scanner.Position) *MakeString {
+func SimpleMakeString(s string, pos Pos) *MakeString {
 	return &MakeString{
-		Pos:     pos,
+		StringPos: pos,
 		Strings:   []string{s},
 	}
 }
 
+func (ms *MakeString) Pos() Pos {
+	return ms.StringPos
+}
+
+func (ms *MakeString) End() Pos {
+	pos := ms.StringPos
+	if len(ms.Strings) > 1 {
+		pos = ms.Variables[len(ms.Variables)-1].End()
+	}
+	return Pos(int(pos) + len(ms.Strings[len(ms.Strings)-1]))
+}
+
 func (ms *MakeString) appendString(s string) {
 	if len(ms.Strings) == 0 {
 		ms.Strings = []string{s}
@@ -97,7 +108,7 @@ func (ms *MakeString) Split(sep string) []*MakeString {
 func (ms *MakeString) SplitN(sep string, n int) []*MakeString {
 	ret := []*MakeString{}
 
-	curMs := SimpleMakeString("", ms.Pos)
+	curMs := SimpleMakeString("", ms.Pos())
 
 	var i int
 	var s string
@@ -115,7 +126,7 @@ func (ms *MakeString) SplitN(sep string, n int) []*MakeString {
 
 			for _, r := range split[1:] {
 				ret = append(ret, curMs)
-				curMs = SimpleMakeString(r, ms.Pos)
+				curMs = SimpleMakeString(r, ms.Pos())
 			}
 		} else {
 			curMs.appendString(s)
@@ -131,7 +142,9 @@ func (ms *MakeString) SplitN(sep string, n int) []*MakeString {
 }
 
 func (ms *MakeString) TrimLeftSpaces() {
+	l := len(ms.Strings[0])
 	ms.Strings[0] = strings.TrimLeftFunc(ms.Strings[0], unicode.IsSpace)
+	ms.StringPos += Pos(len(ms.Strings[0]) - l)
 }
 
 func (ms *MakeString) TrimRightSpaces() {

androidmk/parser/make_strings_test.go

@@ -3,7 +3,6 @@ package parser
 import (
 	"strings"
 	"testing"
-	"text/scanner"
 )
 
 var splitNTestCases = []struct {
@@ -20,31 +19,31 @@ var splitNTestCases = []struct {
 				" h i j",
 			},
 			Variables: []Variable{
-				Variable{Name: SimpleMakeString("var1", scanner.Position{})},
+				Variable{Name: SimpleMakeString("var1", NoPos)},
-				Variable{Name: SimpleMakeString("var2", scanner.Position{})},
+				Variable{Name: SimpleMakeString("var2", NoPos)},
 			},
 		},
 		sep: " ",
 		n:   -1,
 		expected: []*MakeString{
-			SimpleMakeString("a", scanner.Position{}),
+			SimpleMakeString("a", NoPos),
-			SimpleMakeString("b", scanner.Position{}),
+			SimpleMakeString("b", NoPos),
 			&MakeString{
 				Strings: []string{"c", "d"},
 				Variables: []Variable{
-					Variable{Name: SimpleMakeString("var1", scanner.Position{})},
+					Variable{Name: SimpleMakeString("var1", NoPos)},
 				},
 			},
-			SimpleMakeString("e", scanner.Position{}),
+			SimpleMakeString("e", NoPos),
 			&MakeString{
 				Strings: []string{"f", ""},
 				Variables: []Variable{
-					Variable{Name: SimpleMakeString("var2", scanner.Position{})},
+					Variable{Name: SimpleMakeString("var2", NoPos)},
 				},
 			},
-			SimpleMakeString("h", scanner.Position{}),
+			SimpleMakeString("h", NoPos),
-			SimpleMakeString("i", scanner.Position{}),
+			SimpleMakeString("i", NoPos),
-			SimpleMakeString("j", scanner.Position{}),
+			SimpleMakeString("j", NoPos),
 		},
 	},
 	{
@@ -55,20 +54,20 @@ var splitNTestCases = []struct {
 				" h i j",
 			},
 			Variables: []Variable{
-				Variable{Name: SimpleMakeString("var1", scanner.Position{})},
+				Variable{Name: SimpleMakeString("var1", NoPos)},
-				Variable{Name: SimpleMakeString("var2", scanner.Position{})},
+				Variable{Name: SimpleMakeString("var2", NoPos)},
 			},
 		},
 		sep: " ",
 		n:   3,
 		expected: []*MakeString{
-			SimpleMakeString("a", scanner.Position{}),
+			SimpleMakeString("a", NoPos),
-			SimpleMakeString("b", scanner.Position{}),
+			SimpleMakeString("b", NoPos),
 			&MakeString{
 				Strings: []string{"c", "d e f", " h i j"},
 				Variables: []Variable{
-					Variable{Name: SimpleMakeString("var1", scanner.Position{})},
+					Variable{Name: SimpleMakeString("var1", NoPos)},
-					Variable{Name: SimpleMakeString("var2", scanner.Position{})},
+					Variable{Name: SimpleMakeString("var2", NoPos)},
 				},
 			},
 		},

androidmk/parser/makething.go

@@ -1,142 +0,0 @@
-package parser
-
-import (
-	"text/scanner"
-)
-
-type MakeThing interface {
-	AsAssignment() (Assignment, bool)
-	AsComment() (Comment, bool)
-	AsDirective() (Directive, bool)
-	AsRule() (Rule, bool)
-	AsVariable() (Variable, bool)
-	Dump() string
-	Pos() scanner.Position
-	EndPos() scanner.Position
-}
-
-type Assignment struct {
-	makeThing
-	Name   *MakeString
-	Value  *MakeString
-	Target *MakeString
-	Type   string
-}
-
-type Comment struct {
-	makeThing
-	Comment string
-}
-
-type Directive struct {
-	makeThing
-	Name string
-	Args *MakeString
-}
-
-type Rule struct {
-	makeThing
-	Target        *MakeString
-	Prerequisites *MakeString
-	Recipe        string
-}
-
-type Variable struct {
-	makeThing
-	Name *MakeString
-}
-
-type makeThing struct {
-	pos    scanner.Position
-	endPos scanner.Position
-}
-
-func (m makeThing) Pos() scanner.Position {
-	return m.pos
-}
-
-func (m makeThing) EndPos() scanner.Position {
-	return m.endPos
-}
-
-func (makeThing) AsAssignment() (a Assignment, ok bool) {
-	return
-}
-
-func (a Assignment) AsAssignment() (Assignment, bool) {
-	return a, true
-}
-
-func (a Assignment) Dump() string {
-	target := ""
-	if a.Target != nil {
-		target = a.Target.Dump() + ": "
-	}
-	return target + a.Name.Dump() + a.Type + a.Value.Dump()
-}
-
-func (makeThing) AsComment() (c Comment, ok bool) {
-	return
-}
-
-func (c Comment) AsComment() (Comment, bool) {
-	return c, true
-}
-
-func (c Comment) Dump() string {
-	return "#" + c.Comment
-}
-
-func (makeThing) AsDirective() (d Directive, ok bool) {
-	return
-}
-
-func (d Directive) AsDirective() (Directive, bool) {
-	return d, true
-}
-
-func (d Directive) Dump() string {
-	return d.Name + " " + d.Args.Dump()
-}
-
-func (makeThing) AsRule() (r Rule, ok bool) {
-	return
-}
-
-func (r Rule) AsRule() (Rule, bool) {
-	return r, true
-}
-
-func (r Rule) Dump() string {
-	recipe := ""
-	if r.Recipe != "" {
-		recipe = "\n" + r.Recipe
-	}
-	return "rule:       " + r.Target.Dump() + ": " + r.Prerequisites.Dump() + recipe
-}
-
-func (makeThing) AsVariable() (v Variable, ok bool) {
-	return
-}
-
-func (v Variable) AsVariable() (Variable, bool) {
-	return v, true
-}
-
-func (v Variable) Dump() string {
-	return "$(" + v.Name.Dump() + ")"
-}
-
-type byPosition []MakeThing
-
-func (s byPosition) Len() int {
-	return len(s)
-}
-
-func (s byPosition) Swap(i, j int) {
-	s[i], s[j] = s[j], s[i]
-}
-
-func (s byPosition) Less(i, j int) bool {
-	return s[i].Pos().Offset < s[j].Pos().Offset
-}

androidmk/parser/parser.go

@@ -21,7 +21,7 @@ func (e *ParseError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Pos, e.Err)
 }
 
-func (p *parser) Parse() ([]MakeThing, []error) {
+func (p *parser) Parse() ([]Node, []error) {
 	defer func() {
 		if r := recover(); r != nil {
 			if r == errTooManyErrors {
@@ -33,22 +33,24 @@ func (p *parser) Parse() ([]MakeThing, []error) {
 
 	p.parseLines()
 	p.accept(scanner.EOF)
-	p.things = append(p.things, p.comments...)
+	p.nodes = append(p.nodes, p.comments...)
-	sort.Sort(byPosition(p.things))
+	sort.Sort(byPosition(p.nodes))
 
-	return p.things, p.errors
+	return p.nodes, p.errors
 }
 
 type parser struct {
 	scanner  scanner.Scanner
 	tok      rune
 	errors   []error
-	comments []MakeThing
+	comments []Node
-	things   []MakeThing
+	nodes    []Node
+	lines    []int
 }
 
 func NewParser(filename string, r io.Reader) *parser {
 	p := &parser{}
+	p.lines = []int{0}
 	p.scanner.Init(r)
 	p.scanner.Error = func(sc *scanner.Scanner, msg string) {
 		p.errorf(msg)
@@ -65,14 +67,29 @@ func NewParser(filename string, r io.Reader) *parser {
 	return p
 }
 
-func (p *parser) errorf(format string, args ...interface{}) {
+func (p *parser) Unpack(pos Pos) scanner.Position {
+	offset := int(pos)
+	line := sort.Search(len(p.lines), func(i int) bool { return p.lines[i] > offset }) - 1
+	return scanner.Position{
+		Filename: p.scanner.Filename,
+		Line:     line + 1,
+		Column:   offset - p.lines[line] + 1,
+		Offset:   offset,
+	}
+}
+
+func (p *parser) pos() Pos {
 	pos := p.scanner.Position
 	if !pos.IsValid() {
 		pos = p.scanner.Pos()
 	}
+	return Pos(pos.Offset)
+}
+
+func (p *parser) errorf(format string, args ...interface{}) {
 	err := &ParseError{
 		Err: fmt.Errorf(format, args...),
-		Pos: pos,
+		Pos: p.scanner.Position,
 	}
 	p.errors = append(p.errors, err)
 	if len(p.errors) >= maxErrors {
@@ -99,7 +116,9 @@ func (p *parser) next() {
 			p.tok = p.scanner.Scan()
 		}
 	}
-	return
+	if p.tok == '\n' {
+		p.lines = append(p.lines, p.scanner.Position.Offset+1)
+	}
 }
 
 func (p *parser) parseLines() {
@@ -110,7 +129,7 @@ func (p *parser) parseLines() {
 			continue
 		}
 
-		ident, _ := p.parseExpression('=', '?', ':', '#', '\n')
+		ident := p.parseExpression('=', '?', ':', '#', '\n')
 
 		p.ignoreSpaces()
 
@@ -142,7 +161,7 @@ func (p *parser) parseLines() {
 		case '#', '\n', scanner.EOF:
 			ident.TrimRightSpaces()
 			if v, ok := toVariable(ident); ok {
-				p.things = append(p.things, v)
+				p.nodes = append(p.nodes, &v)
 			} else if !ident.Empty() {
 				p.errorf("expected directive, rule, or assignment after ident " + ident.Dump())
 			}
@@ -168,9 +187,9 @@ func (p *parser) parseDirective() bool {
 	}
 
 	d := p.scanner.TokenText()
-	pos := p.scanner.Position
+	pos := p.pos()
-	endPos := pos
 	p.accept(scanner.Ident)
+	endPos := NoPos
 
 	expression := SimpleMakeString("", pos)
 
@@ -178,35 +197,33 @@ func (p *parser) parseDirective() bool {
 	case "endif", "endef", "else":
 		// Nothing
 	case "define":
-		expression = p.parseDefine()
+		expression, endPos = p.parseDefine()
 	default:
 		p.ignoreSpaces()
-		expression, endPos = p.parseExpression()
+		expression = p.parseExpression()
 	}
 
-	p.things = append(p.things, Directive{
+	p.nodes = append(p.nodes, &Directive{
-		makeThing: makeThing{
+		NamePos: pos,
-			pos:    pos,
-			endPos: endPos,
-		},
 		Name:    d,
 		Args:    expression,
+		EndPos:  endPos,
 	})
 	return true
 }
 
-func (p *parser) parseDefine() *MakeString {
+func (p *parser) parseDefine() (*MakeString, Pos) {
-	value := SimpleMakeString("", p.scanner.Position)
+	value := SimpleMakeString("", p.pos())
 
 loop:
 	for {
 		switch p.tok {
 		case scanner.Ident:
+			value.appendString(p.scanner.TokenText())
 			if p.scanner.TokenText() == "endef" {
 				p.accept(scanner.Ident)
 				break loop
 			}
-			value.appendString(p.scanner.TokenText())
 			p.accept(scanner.Ident)
 		case '\\':
 			p.parseEscape()
@@ -235,7 +252,7 @@ loop:
 		}
 	}
 
-	return value
+	return value, p.pos()
 }
 
 func (p *parser) parseEscape() {
@@ -244,8 +261,8 @@ func (p *parser) parseEscape() {
 	p.scanner.Mode = scanner.ScanIdents
 }
 
-func (p *parser) parseExpression(end ...rune) (*MakeString, scanner.Position) {
+func (p *parser) parseExpression(end ...rune) *MakeString {
-	value := SimpleMakeString("", p.scanner.Position)
+	value := SimpleMakeString("", p.pos())
 
 	endParen := false
 	for _, r := range end {
@@ -255,14 +272,11 @@ func (p *parser) parseExpression(end ...rune) (*MakeString, scanner.Position) {
 	}
 	parens := 0
 
-	endPos := p.scanner.Position
-
 loop:
 	for {
 		if endParen && parens > 0 && p.tok == ')' {
 			parens--
 			value.appendString(")")
-			endPos = p.scanner.Position
 			p.accept(')')
 			continue
 		}
@@ -278,7 +292,6 @@ loop:
 			break loop
 		case scanner.Ident:
 			value.appendString(p.scanner.TokenText())
-			endPos = p.scanner.Position
 			p.accept(scanner.Ident)
 		case '\\':
 			p.parseEscape()
@@ -288,18 +301,17 @@ loop:
 			case scanner.EOF:
 				p.errorf("expected escaped character, found %s",
 					scanner.TokenString(p.tok))
-				return value, endPos
+				return value
 			default:
 				value.appendString(`\` + string(p.tok))
 			}
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		case '#':
 			p.parseComment()
 			break loop
 		case '$':
 			var variable Variable
-			variable, endPos = p.parseVariable()
+			variable = p.parseVariable()
 			value.appendVariable(variable)
 		case scanner.EOF:
 			break loop
@@ -308,11 +320,9 @@ loop:
 				parens++
 			}
 			value.appendString("(")
-			endPos = p.scanner.Position
 			p.accept('(')
 		default:
 			value.appendString(p.scanner.TokenText())
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		}
 	}
@@ -320,12 +330,11 @@ loop:
 	if parens > 0 {
 		p.errorf("expected closing paren %s", value.Dump())
 	}
-	return value, endPos
+	return value
 }
 
-func (p *parser) parseVariable() (Variable, scanner.Position) {
+func (p *parser) parseVariable() Variable {
-	pos := p.scanner.Position
+	pos := p.pos()
-	endPos := pos
 	p.accept('$')
 	var name *MakeString
 	switch p.tok {
@@ -334,30 +343,26 @@ func (p *parser) parseVariable() (Variable, scanner.Position) {
 	case '{':
 		return p.parseBracketedVariable('{', '}', pos)
 	case '$':
-		name = SimpleMakeString("__builtin_dollar", scanner.Position{})
+		name = SimpleMakeString("__builtin_dollar", NoPos)
 	case scanner.EOF:
 		p.errorf("expected variable name, found %s",
 			scanner.TokenString(p.tok))
 	default:
-		name, endPos = p.parseExpression(variableNameEndRunes...)
+		name = p.parseExpression(variableNameEndRunes...)
 	}
 
-	return p.nameToVariable(name, pos, endPos), endPos
+	return p.nameToVariable(name)
 }
 
-func (p *parser) parseBracketedVariable(start, end rune, pos scanner.Position) (Variable, scanner.Position) {
+func (p *parser) parseBracketedVariable(start, end rune, pos Pos) Variable {
 	p.accept(start)
-	name, endPos := p.parseExpression(end)
+	name := p.parseExpression(end)
 	p.accept(end)
-	return p.nameToVariable(name, pos, endPos), endPos
+	return p.nameToVariable(name)
 }
 
-func (p *parser) nameToVariable(name *MakeString, pos, endPos scanner.Position) Variable {
+func (p *parser) nameToVariable(name *MakeString) Variable {
 	return Variable{
-		makeThing: makeThing{
-			pos:    pos,
-			endPos: endPos,
-		},
 		Name: name,
 	}
 }
@@ -366,12 +371,11 @@ func (p *parser) parseRule(target *MakeString) {
 	prerequisites, newLine := p.parseRulePrerequisites(target)
 
 	recipe := ""
-	endPos := p.scanner.Position
+	recipePos := p.pos()
 loop:
 	for {
 		if newLine {
 			if p.tok == '\t' {
-				endPos = p.scanner.Position
 				p.accept('\t')
 				newLine = false
 				continue loop
@@ -388,31 +392,25 @@ loop:
 		case '\\':
 			p.parseEscape()
 			recipe += string(p.tok)
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		case '\n':
 			newLine = true
 			recipe += "\n"
-			endPos = p.scanner.Position
 			p.accept('\n')
 		case scanner.EOF:
 			break loop
 		default:
 			recipe += p.scanner.TokenText()
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		}
 	}
 
 	if prerequisites != nil {
-		p.things = append(p.things, Rule{
+		p.nodes = append(p.nodes, &Rule{
-			makeThing: makeThing{
-				pos:    target.Pos,
-				endPos: endPos,
-			},
 			Target:        target,
 			Prerequisites: prerequisites,
 			Recipe:        recipe,
+			RecipePos:     recipePos,
 		})
 	}
 }
@@ -422,7 +420,7 @@ func (p *parser) parseRulePrerequisites(target *MakeString) (*MakeString, bool)
 
 	p.ignoreSpaces()
 
-	prerequisites, _ := p.parseExpression('#', '\n', ';', ':', '=')
+	prerequisites := p.parseExpression('#', '\n', ';', ':', '=')
 
 	switch p.tok {
 	case '\n':
@@ -439,7 +437,7 @@ func (p *parser) parseRulePrerequisites(target *MakeString) (*MakeString, bool)
 			p.parseAssignment(":=", target, prerequisites)
 			return nil, true
 		} else {
-			more, _ := p.parseExpression('#', '\n', ';')
+			more := p.parseExpression('#', '\n', ';')
 			prerequisites.appendMakeString(more)
 		}
 	case '=':
@@ -453,10 +451,9 @@ func (p *parser) parseRulePrerequisites(target *MakeString) (*MakeString, bool)
 }
 
 func (p *parser) parseComment() {
-	pos := p.scanner.Position
+	pos := p.pos()
 	p.accept('#')
 	comment := ""
-	endPos := pos
 loop:
 	for {
 		switch p.tok {
@@ -467,26 +464,20 @@ loop:
 			} else {
 				comment += "\\" + p.scanner.TokenText()
 			}
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		case '\n':
-			endPos = p.scanner.Position
 			p.accept('\n')
 			break loop
 		case scanner.EOF:
 			break loop
 		default:
 			comment += p.scanner.TokenText()
-			endPos = p.scanner.Position
 			p.accept(p.tok)
 		}
 	}
 
-	p.comments = append(p.comments, Comment{
+	p.comments = append(p.comments, &Comment{
-		makeThing: makeThing{
+		CommentPos: pos,
-			pos:    pos,
-			endPos: endPos,
-		},
 		Comment:    comment,
 	})
 }
@@ -496,7 +487,7 @@ func (p *parser) parseAssignment(t string, target *MakeString, ident *MakeString
 	// non-whitespace character after the = until the end of the logical line,
 	// which may included escaped newlines
 	p.accept('=')
-	value, endPos := p.parseExpression()
+	value := p.parseExpression()
 	value.TrimLeftSpaces()
 	if ident.EndsWith('+') && t == "=" {
 		ident.TrimRightOne()
@@ -505,11 +496,7 @@ func (p *parser) parseAssignment(t string, target *MakeString, ident *MakeString
 
 	ident.TrimRightSpaces()
 
-	p.things = append(p.things, Assignment{
+	p.nodes = append(p.nodes, &Assignment{
-		makeThing: makeThing{
-			pos:    ident.Pos,
-			endPos: endPos,
-		},
 		Name:   ident,
 		Value:  value,
 		Target: target,