From 106aa941dfda45d4aa5235b12317124aaf8941a0 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Sat, 9 Jan 2021 23:12:56 -0800 Subject: [PATCH] [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 TryBot-Result: Go Bot Reviewed-by: Robert Griesemer Trust: Robert Griesemer Trust: Matthew Dempsky --- src/cmd/compile/internal/dwarfgen/marker.go | 94 +++++++++++++++++++++ src/cmd/compile/internal/noder/noder.go | 79 +++++++---------- 2 files changed, 123 insertions(+), 50 deletions(-) create mode 100644 src/cmd/compile/internal/dwarfgen/marker.go diff --git a/src/cmd/compile/internal/dwarfgen/marker.go b/src/cmd/compile/internal/dwarfgen/marker.go new file mode 100644 index 00000000000..ec6ce45a900 --- /dev/null +++ b/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] +} diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index b3f3c23c29d..71a5df082bc 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/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 - // current function at the moment each open scope was opened. - trackScopes bool - scopeVars []int + funcState *funcState // 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 - p.scope = 0 + outerFuncState := p.funcState + 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) - p.scopeVars = append(p.scopeVars, len(ir.CurFunc.Dcl)) - p.scope = ir.ScopeID(len(ir.CurFunc.Parents)) - - p.markScope(pos) + fs.scopeVars = append(fs.scopeVars, len(ir.CurFunc.Dcl)) + fs.marker.Push(p.makeXPos(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] - p.scopeVars = p.scopeVars[:len(p.scopeVars)-1] + scopeVars := fs.scopeVars[len(fs.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. - - if int(p.scope) != len(ir.CurFunc.Parents) { - base.Fatalf("scope tracking inconsistency, no variables declared but scopes were not retracted") - } - - 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 + fs.marker.Unpush() + } else { + fs.marker.Pop(p.makeXPos(pos)) } - - 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.