From ba9c80143385b1db976a755b074bb94cd30699ad Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 11 Mar 2014 18:24:39 -0400 Subject: [PATCH] go.tools: various comments + doc tweaks. No functional changes. LGTM=gri R=gri CC=golang-codereviews https://golang.org/cl/74270043 --- go/callgraph/callgraph.go | 1 + go/callgraph/util.go | 4 ++++ go/pointer/gen.go | 4 ++++ go/pointer/labels.go | 5 +++-- go/ssa/const.go | 1 + go/ssa/create.go | 2 ++ go/ssa/ssa.go | 7 ++++--- go/types/methodset.go | 2 +- oracle/implements.go | 2 +- oracle/oracle.go | 2 +- oracle/peers.go | 5 +++-- 11 files changed, 25 insertions(+), 10 deletions(-) diff --git a/go/callgraph/callgraph.go b/go/callgraph/callgraph.go index 5b4e3487..7d78a05e 100644 --- a/go/callgraph/callgraph.go +++ b/go/callgraph/callgraph.go @@ -100,6 +100,7 @@ func (e Edge) String() string { } // AddEdge adds the edge (caller, site, callee) to the call graph. +// Elimination of duplicate edges is the caller's responsibility. func AddEdge(caller *Node, site ssa.CallInstruction, callee *Node) { e := &Edge{caller, site, callee} callee.In = append(callee.In, e) diff --git a/go/callgraph/util.go b/go/callgraph/util.go index f807dfd4..1003fcda 100644 --- a/go/callgraph/util.go +++ b/go/callgraph/util.go @@ -82,6 +82,10 @@ func PathSearch(start *Node, isEnd func(*Node) bool) []*Edge { // synthetic functions (except g.Root and package initializers), // preserving the topology. func (g *Graph) DeleteSyntheticNodes() { + // TODO(adonovan): opt: this step results in duplicate + // edges---approx 10% of the total. I suspect this is due to + // some interface method calls dispatching to both (C).f and + // (*C).f where the latter is a wrapper. for fn, cgn := range g.Nodes { if cgn == g.Root || fn.Synthetic == "" || isInit(cgn.Func) { continue // keep diff --git a/go/pointer/gen.go b/go/pointer/gen.go index 77cf1596..fdd2370e 100644 --- a/go/pointer/gen.go +++ b/go/pointer/gen.go @@ -1103,6 +1103,10 @@ func (a *analysis) genRootCalls() *cgnode { r.String() // (asserts that it doesn't crash) root := a.makeCGNode(r, 0, nil) + // TODO(adonovan): make an ssa utility to construct an actual + // root function so we don't need to special-case site-less + // call edges. + // For each main package, call main.init(), main.main(). for _, mainPkg := range a.config.Mains { main := mainPkg.Func("main") diff --git a/go/pointer/labels.go b/go/pointer/labels.go index 25f64921..3fd8681b 100644 --- a/go/pointer/labels.go +++ b/go/pointer/labels.go @@ -14,7 +14,7 @@ import ( ) // A Label is an entity that may be pointed to by a pointer, map, -// channel, 'func', slice or interface. Labels include: +// channel, 'func', slice or interface. // // Labels include: // - functions @@ -79,8 +79,9 @@ func (l Label) Pos() token.Pos { // String returns the printed form of this label. // // Examples: Object type: +// x (a variable) // (sync.Mutex).Lock (a function) -// "foo":[]byte (a slice constant) +// convert (array created by conversion) // makemap (map allocated via make) // makechan (channel allocated via make) // makeinterface (tagged object allocated by makeinterface) diff --git a/go/ssa/const.go b/go/ssa/const.go index bb1eb25f..36ae99ea 100644 --- a/go/ssa/const.go +++ b/go/ssa/const.go @@ -73,6 +73,7 @@ func (c *Const) RelString(from *types.Package) string { } else if c.Value.Kind() == exact.String { s = exact.StringVal(c.Value) const max = 20 + // TODO(adonovan): don't cut a rune in half. if len(s) > max { s = s[:max-3] + "..." // abbreviate } diff --git a/go/ssa/create.go b/go/ssa/create.go index cab8ba81..3317013a 100644 --- a/go/ssa/create.go +++ b/go/ssa/create.go @@ -48,6 +48,8 @@ func Create(iprog *loader.Program, mode BuilderMode) *Program { } for _, info := range iprog.AllPackages { + // TODO(adonovan): relax this constraint if the + // program contains only "soft" errors. if info.TransitivelyErrorFree { prog.CreatePackage(info) } diff --git a/go/ssa/ssa.go b/go/ssa/ssa.go index a9ace79b..0b9deec8 100644 --- a/go/ssa/ssa.go +++ b/go/ssa/ssa.go @@ -312,11 +312,11 @@ type Function struct { // The tree may be navigated using Idom()/Dominees() and queried using // Dominates(). // -// The order of Preds and Succs are significant (to Phi and If +// The order of Preds and Succs is significant (to Phi and If // instructions, respectively). // type BasicBlock struct { - Index int // index of this block within Func.Blocks + Index int // index of this block within Parent().Blocks Comment string // optional label; no semantic significance parent *Function // parent function Instrs []Instruction // instructions in order @@ -521,6 +521,7 @@ type BinOp struct { // MUL is pointer indirection (load). // XOR is bitwise complement. // SUB is negation. +// NOT is logical negation. // // If CommaOk and Op=ARROW, the result is a 2-tuple of the value above // and a boolean indicating the success of the receive. The @@ -689,7 +690,7 @@ type MakeChan struct { // Both Len and Cap must be non-nil Values of integer type. // // (Alloc(types.Array) followed by Slice will not suffice because -// Alloc can only create arrays of statically known length.) +// Alloc can only create arrays of constant length.) // // Type() returns a (possibly named) *types.Slice. // diff --git a/go/types/methodset.go b/go/types/methodset.go index 9ded496e..77297ea0 100644 --- a/go/types/methodset.go +++ b/go/types/methodset.go @@ -13,7 +13,7 @@ import ( ) // A MethodSet is an ordered set of concrete or abstract (interface) methods; -// a method is a MethodVal selection. +// a method is a MethodVal selection, and they are ordered by ascending m.Obj().Id(). // The zero value for a MethodSet is a ready-to-use empty method set. type MethodSet struct { list []*Selection diff --git a/oracle/implements.go b/oracle/implements.go index 423de543..3dcf42ee 100644 --- a/oracle/implements.go +++ b/oracle/implements.go @@ -97,7 +97,7 @@ func implements(o *Oracle, qpos *QueryPos) (queryResult, error) { pos = nt.Obj() } - // Sort types (arbitrarily) to ensure test nondeterminism. + // Sort types (arbitrarily) to ensure test determinism. sort.Sort(typesByString(to)) sort.Sort(typesByString(from)) sort.Sort(typesByString(fromPtr)) diff --git a/oracle/oracle.go b/oracle/oracle.go index 4d08027d..67aac5cb 100644 --- a/oracle/oracle.go +++ b/oracle/oracle.go @@ -162,7 +162,7 @@ type Result struct { fset *token.FileSet q queryResult // the query-specific result mode string // query mode - warnings []pointer.Warning // pointer analysis warnings + warnings []pointer.Warning // pointer analysis warnings (TODO(adonovan): fix: never populated!) } // Serial returns an instance of serial.Result, which implements the diff --git a/oracle/peers.go b/oracle/peers.go index fffc27a0..d4c910d5 100644 --- a/oracle/peers.go +++ b/oracle/peers.go @@ -19,9 +19,10 @@ import ( // peers enumerates, for a given channel send (or receive) operation, // the set of possible receives (or sends) that correspond to it. // -// TODO(adonovan): support reflect.{Select,Recv,Send}. +// TODO(adonovan): support reflect.{Select,Recv,Send,Close}. // TODO(adonovan): permit the user to query based on a MakeChan (not send/recv), // or the implicit receive in "for v := range ch". +// TODO(adonovan): support "close" as a channel op. // func peers(o *Oracle, qpos *QueryPos) (queryResult, error) { arrowPos := findArrow(qpos) @@ -132,7 +133,7 @@ type chanOp struct { // chanOps returns a slice of all the channel operations in the instruction. func chanOps(instr ssa.Instruction) []chanOp { - // TODO(adonovan): handle calls to reflect.{Select,Recv,Send} too. + // TODO(adonovan): handle calls to reflect.{Select,Recv,Send,Close} too. var ops []chanOp switch instr := instr.(type) { case *ssa.UnOp: