[dev.typeparams] cmd/compile: refactor DWARF scope marking

This CL extracts and simplifies noder's DWARF scope tracking code to
make it easier for reuse by irgen.

The previous code tried to be really clever about avoid recording
multiple scope boundaries at the same position (as happens at the end
of "if" and "for" statements). I had a really hard time remember how
this code worked exactly, so I've reimplemented a simpler algorithm
that just tracks all scope marks, and then compacts them at the end
before saving them to the ir.Func.

Passes toolstash -cmp.

Change-Id: Ibeb37997b77dc5179360d7db557c82ae1682e127
Reviewed-on: https://go-review.googlesource.com/c/go/+/282918
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Griesemer <gri@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>

src/cmd/compile/internal/dwarfgen/marker.go

@@ -0,0 +1,94 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package dwarfgen
+
+import (
+	"cmd/compile/internal/base"
+	"cmd/compile/internal/ir"
+	"cmd/internal/src"
+)
+
+// A ScopeMarker tracks scope nesting and boundaries for later use
+// during DWARF generation.
+type ScopeMarker struct {
+	parents []ir.ScopeID
+	marks   []ir.Mark
+}
+
+// checkPos validates the given position and returns the current scope.
+func (m *ScopeMarker) checkPos(pos src.XPos) ir.ScopeID {
+	if !pos.IsKnown() {
+		base.Fatalf("unknown scope position")
+	}
+
+	if len(m.marks) == 0 {
+		return 0
+	}
+
+	last := &m.marks[len(m.marks)-1]
+	if xposBefore(pos, last.Pos) {
+		base.FatalfAt(pos, "non-monotonic scope positions\n\t%v: previous scope position", base.FmtPos(last.Pos))
+	}
+	return last.Scope
+}
+
+// Push records a transition to a new child scope of the current scope.
+func (m *ScopeMarker) Push(pos src.XPos) {
+	current := m.checkPos(pos)
+
+	m.parents = append(m.parents, current)
+	child := ir.ScopeID(len(m.parents))
+
+	m.marks = append(m.marks, ir.Mark{Pos: pos, Scope: child})
+}
+
+// Pop records a transition back to the current scope's parent.
+func (m *ScopeMarker) Pop(pos src.XPos) {
+	current := m.checkPos(pos)
+
+	parent := m.parents[current-1]
+
+	m.marks = append(m.marks, ir.Mark{Pos: pos, Scope: parent})
+}
+
+// Unpush removes the current scope, which must be empty.
+func (m *ScopeMarker) Unpush() {
+	i := len(m.marks) - 1
+	current := m.marks[i].Scope
+
+	if current != ir.ScopeID(len(m.parents)) {
+		base.FatalfAt(m.marks[i].Pos, "current scope is not empty")
+	}
+
+	m.parents = m.parents[:current-1]
+	m.marks = m.marks[:i]
+}
+
+// WriteTo writes the recorded scope marks to the given function,
+// and resets the marker for reuse.
+func (m *ScopeMarker) WriteTo(fn *ir.Func) {
+	m.compactMarks()
+
+	fn.Parents = make([]ir.ScopeID, len(m.parents))
+	copy(fn.Parents, m.parents)
+	m.parents = m.parents[:0]
+
+	fn.Marks = make([]ir.Mark, len(m.marks))
+	copy(fn.Marks, m.marks)
+	m.marks = m.marks[:0]
+}
+
+func (m *ScopeMarker) compactMarks() {
+	n := 0
+	for _, next := range m.marks {
+		if n > 0 && next.Pos == m.marks[n-1].Pos {
+			m.marks[n-1].Scope = next.Scope
+			continue
+		}
+		m.marks[n] = next
+		n++
+	}
+	m.marks = m.marks[:n]
+}

src/cmd/compile/internal/noder/noder.go

@@ -19,6 +19,7 @@ import (
 	"unicode/utf8"
 
 	"cmd/compile/internal/base"
+	"cmd/compile/internal/dwarfgen"
 	"cmd/compile/internal/importer"
 	"cmd/compile/internal/ir"
 	"cmd/compile/internal/syntax"
@@ -292,22 +293,17 @@ type noder struct {
 	linknames      []linkname
 	pragcgobuf     [][]string
 	err            chan syntax.Error
-	scope          ir.ScopeID
 	importedUnsafe bool
 	importedEmbed  bool
+	trackScopes    bool
 
-	// scopeVars is a stack tracking the number of variables declared in the
+	funcState *funcState
-	// current function at the moment each open scope was opened.
-	trackScopes bool
-	scopeVars   []int
 
 	// typeInfo provides access to the type information computed by the new
 	// typechecker. It is only present if -G is set, and all noders point to
 	// the same types.Info. For now this is a local field, if need be we can
 	// make it global.
 	typeInfo *types2.Info
-
-	lastCloseScopePos syntax.Pos
 }
 
 // For now we provide these basic accessors to get to type and object
@@ -335,9 +331,20 @@ func (p *noder) sel(x *syntax.SelectorExpr) *types2.Selection {
 	return p.typeInfo.Selections[x]
 }
 
+// funcState tracks all per-function state to make handling nested
+// functions easier.
+type funcState struct {
+	// scopeVars is a stack tracking the number of variables declared in
+	// the current function at the moment each open scope was opened.
+	scopeVars []int
+	marker    dwarfgen.ScopeMarker
+
+	lastCloseScopePos syntax.Pos
+}
+
 func (p *noder) funcBody(fn *ir.Func, block *syntax.BlockStmt) {
-	oldScope := p.scope
+	outerFuncState := p.funcState
-	p.scope = 0
+	p.funcState = new(funcState)
 	typecheck.StartFuncBody(fn)
 
 	if block != nil {
@@ -352,62 +359,34 @@ func (p *noder) funcBody(fn *ir.Func, block *syntax.BlockStmt) {
 	}
 
 	typecheck.FinishFuncBody()
-	p.scope = oldScope
+	p.funcState.marker.WriteTo(fn)
+	p.funcState = outerFuncState
 }
 
 func (p *noder) openScope(pos syntax.Pos) {
+	fs := p.funcState
 	types.Markdcl()
 
 	if p.trackScopes {
-		ir.CurFunc.Parents = append(ir.CurFunc.Parents, p.scope)
+		fs.scopeVars = append(fs.scopeVars, len(ir.CurFunc.Dcl))
-		p.scopeVars = append(p.scopeVars, len(ir.CurFunc.Dcl))
+		fs.marker.Push(p.makeXPos(pos))
-		p.scope = ir.ScopeID(len(ir.CurFunc.Parents))
-
-		p.markScope(pos)
 	}
 }
 
 func (p *noder) closeScope(pos syntax.Pos) {
-	p.lastCloseScopePos = pos
+	fs := p.funcState
+	fs.lastCloseScopePos = pos
 	types.Popdcl()
 
 	if p.trackScopes {
-		scopeVars := p.scopeVars[len(p.scopeVars)-1]
+		scopeVars := fs.scopeVars[len(fs.scopeVars)-1]
-		p.scopeVars = p.scopeVars[:len(p.scopeVars)-1]
+		fs.scopeVars = fs.scopeVars[:len(fs.scopeVars)-1]
 		if scopeVars == len(ir.CurFunc.Dcl) {
 			// no variables were declared in this scope, so we can retract it.
-
+			fs.marker.Unpush()
-			if int(p.scope) != len(ir.CurFunc.Parents) {
+		} else {
-				base.Fatalf("scope tracking inconsistency, no variables declared but scopes were not retracted")
+			fs.marker.Pop(p.makeXPos(pos))
-			}
-
-			p.scope = ir.CurFunc.Parents[p.scope-1]
-			ir.CurFunc.Parents = ir.CurFunc.Parents[:len(ir.CurFunc.Parents)-1]
-
-			nmarks := len(ir.CurFunc.Marks)
-			ir.CurFunc.Marks[nmarks-1].Scope = p.scope
-			prevScope := ir.ScopeID(0)
-			if nmarks >= 2 {
-				prevScope = ir.CurFunc.Marks[nmarks-2].Scope
-			}
-			if ir.CurFunc.Marks[nmarks-1].Scope == prevScope {
-				ir.CurFunc.Marks = ir.CurFunc.Marks[:nmarks-1]
-			}
-			return
 		}
-
-		p.scope = ir.CurFunc.Parents[p.scope-1]
-
-		p.markScope(pos)
-	}
-}
-
-func (p *noder) markScope(pos syntax.Pos) {
-	xpos := p.makeXPos(pos)
-	if i := len(ir.CurFunc.Marks); i > 0 && ir.CurFunc.Marks[i-1].Pos == xpos {
-		ir.CurFunc.Marks[i-1].Scope = p.scope
-	} else {
-		ir.CurFunc.Marks = append(ir.CurFunc.Marks, ir.Mark{Pos: xpos, Scope: p.scope})
 	}
 }
 
@@ -416,7 +395,7 @@ func (p *noder) markScope(pos syntax.Pos) {
 // "if" statements, as their implicit blocks always end at the same
 // position as an explicit block.
 func (p *noder) closeAnotherScope() {
-	p.closeScope(p.lastCloseScopePos)
+	p.closeScope(p.funcState.lastCloseScopePos)
 }
 
 // linkname records a //go:linkname directive.