go.tools/pointer: minor API simplifications.

Details:
- Warnings are reported as values in Result, not a callback in Config.
- remove TODO to eliminate Print callback.  It's better than the alternative.
- remove unused Config.root field.
- hang Result off analysis object (impl. detail)
- reword TODO.

R=crawshaw
CC=golang-dev
https://golang.org/cl/14128043

oracle/oracle.go

@@ -99,12 +99,6 @@ type queryResult interface {
 	display(printf printfFunc)
 }
 
-type warning struct {
-	pos    token.Pos
-	format string
-	args   []interface{}
-}
-
 // A QueryPos represents the position provided as input to a query:
 // a textual extent in the program's source code, the AST node it
 // corresponds to, and the package to which it belongs.
@@ -121,9 +115,9 @@ type Result struct {
 	fset *token.FileSet
 	// fprintf is a closure over the oracle's fileset and start/end position.
 	fprintf  func(w io.Writer, pos interface{}, format string, args ...interface{})
-	q        queryResult // the query-specific result
-	mode     string      // query mode
-	warnings []warning   // pointer analysis warnings
+	q        queryResult       // the query-specific result
+	mode     string            // query mode
+	warnings []pointer.Warning // pointer analysis warnings
 }
 
 // Serial returns an instance of serial.Result, which implements the
@@ -135,8 +129,8 @@ func (res *Result) Serial() *serial.Result {
 	res.q.toSerial(resj, res.fset)
 	for _, w := range res.warnings {
 		resj.Warnings = append(resj.Warnings, serial.PTAWarning{
-			Pos:     res.fset.Position(w.pos).String(),
-			Message: fmt.Sprintf(w.format, w.args...),
+			Pos:     res.fset.Position(w.Pos).String(),
+			Message: w.Message,
 		})
 	}
 	return resj
@@ -302,9 +296,6 @@ func (o *Oracle) query(minfo *modeInfo, qpos *QueryPos) (*Result, error) {
 		fset:    o.prog.Fset,
 		fprintf: o.fprintf, // captures o.prog, o.{start,end}Pos for later printing
 	}
-	o.config.Warn = func(pos token.Pos, format string, args ...interface{}) {
-		res.warnings = append(res.warnings, warning{pos, format, args})
-	}
 	var err error
 	res.q, err = minfo.impl(o, qpos)
 	if err != nil {
@@ -342,7 +333,7 @@ func (res *Result) WriteTo(out io.Writer) {
 	if res.warnings != nil {
 		fmt.Fprintln(out, "\nPointer analysis warnings:")
 		for _, w := range res.warnings {
-			printf(w.pos, "warning: "+w.format, w.args...)
+			printf(w.Pos, "warning: "+w.Message)
 		}
 	}
 }

pointer/TODO

@@ -16,13 +16,11 @@ CONSTRAINT GENERATION:
      A downside is that we can't keep the identity field of struct
      allocations that identifies the object.    
 
-PRESOLVER OPTIMISATIONS
-- use HVN, HRU, LE, PE, HCD, LCD.
-
-SOLVER:
-- use BDDs and/or sparse bitvectors for ptsets
+OPTIMISATIONS
+- pre-solver: PE and LE via HVN/HRU.
+- solver: HCD, LCD.
+- use sparse bitvectors for ptsets
 - use sparse bitvectors for graph edges
-- use BDD-based relational composition for e.g. offset computations.
 - experiment with different worklist algorithms:
    priority queue (solver visit-time order)
    red-black tree (node id order)
@@ -34,9 +32,8 @@ SOLVER:
 API:
 - Some optimisations (e.g. LE, PE) may change the API.
   Think about them sooner rather than later.
-- Eliminate Print probe now that we can query specific ssa.Values.
 
-Misc:
+MISC:
 - os.Args should point to something; currently they don't.
 - Test on all platforms.  
   Currently we assume these go/build tags: linux, amd64, !cgo.

pointer/analysis.go

@@ -183,7 +183,7 @@ type analysis struct {
 	localval    map[ssa.Value]nodeid        // node for each local ssa.Value
 	localobj    map[ssa.Value]nodeid        // maps v to sole member of pts(v), if singleton
 	work        worklist                    // solver's worklist
-	queries     map[ssa.Value][]Pointer     // same as Results.Queries
+	result      *Result                     // results of the analysis
 
 	// Reflection:
 	hasher          typemap.Hasher // cache of type hashes
@@ -221,13 +221,7 @@ func (a *analysis) labelFor(id nodeid) *Label {
 }
 
 func (a *analysis) warnf(pos token.Pos, format string, args ...interface{}) {
-	if Warn := a.config.Warn; Warn != nil {
-		Warn(pos, format, args...)
-	} else {
-		fmt.Fprintf(os.Stderr, "%s: warning: ", a.prog.Fset.Position(pos))
-		fmt.Fprintf(os.Stderr, format, args...)
-		fmt.Fprintln(os.Stderr)
-	}
+	a.result.Warnings = append(a.result.Warnings, Warning{pos, fmt.Sprintf(format, args...)})
 }
 
 // Analyze runs the pointer analysis with the scope and options
@@ -245,7 +239,9 @@ func Analyze(config *Config) *Result {
 		intrinsics:  make(map[*ssa.Function]intrinsic),
 		probes:      make(map[*ssa.CallCommon]nodeid),
 		work:        makeMapWorklist(),
-		queries:     make(map[ssa.Value][]Pointer),
+		result: &Result{
+			Queries: make(map[ssa.Value][]Pointer),
+		},
 	}
 
 	if false {
@@ -318,13 +314,9 @@ func Analyze(config *Config) *Result {
 		}
 	}
 
-	var callgraph *cgraph
 	if a.config.BuildCallGraph {
-		callgraph = &cgraph{root, a.cgnodes}
+		a.result.CallGraph = &cgraph{root, a.cgnodes}
 	}
 
-	return &Result{
-		CallGraph: callgraph,
-		Queries:   a.queries,
-	}
+	return a.result
 }

pointer/api.go

@@ -14,12 +14,12 @@ import (
 	"code.google.com/p/go.tools/ssa"
 )
 
+// A Config formulates a pointer analysis problem for Analyze().
 type Config struct {
-	// -------- Scope of the analysis --------
-
-	// Clients must provide the analysis with at least one package defining a main() function.
-	Mains []*ssa.Package // set of 'main' packages to analyze
-	root  *ssa.Function  // synthetic analysis root
+	// Mains contains the set of 'main' packages to analyze
+	// Clients must provide the analysis with at least one
+	// package defining a main() function.
+	Mains []*ssa.Package
 
 	// Reflection determines whether to handle reflection
 	// operators soundly, which is currently rather slow since it
@@ -32,25 +32,15 @@ type Config struct {
 	// If enabled, the graph will be available in Result.CallGraph.
 	BuildCallGraph bool
 
-	// -------- Optional callbacks invoked by the analysis --------
-
-	// Warn is invoked for each warning encountered by the analysis,
-	// e.g. unknown external function, unsound use of unsafe.Pointer.
-	// pos may be zero if the position is not known.
-	Warn func(pos token.Pos, format string, args ...interface{})
-
 	// Print is invoked during the analysis for each discovered
-	// call to the built-in print(x).
+	// call to the built-in print(x), providing a convenient way
+	// to identify arbitrary expressions of interest in the tests.
 	//
 	// Pointer p may be saved until the analysis is complete, at
 	// which point its methods provide access to the analysis
 	// (The result of callings its methods within the Print
 	// callback is undefined.)  p is nil if x is non-pointerlike.
 	//
-	// TODO(adonovan): this was a stop-gap measure for identifing
-	// arbitrary expressions of interest in the tests.  Now that
-	// ssa.ValueForExpr exists, we should use that instead.
-	//
 	Print func(site *ssa.CallCommon, p Pointer)
 
 	// The client populates Queries[v] for each ssa.Value v of
@@ -73,9 +63,7 @@ type Config struct {
 	//
 	Queries map[ssa.Value]Indirect
 
-	// -------- Other configuration options --------
-
-	// If Log is non-nil, a log messages are written to it.
+	// If Log is non-nil, log messages are written to it.
 	// Logging is extremely verbose.
 	Log io.Writer
 }
@@ -89,6 +77,11 @@ func (c *Config) prog() *ssa.Program {
 	panic("empty scope")
 }
 
+type Warning struct {
+	Pos     token.Pos
+	Message string
+}
+
 // A Result contains the results of a pointer analysis.
 //
 // See Config for how to request the various Result components.
@@ -96,6 +89,7 @@ func (c *Config) prog() *ssa.Program {
 type Result struct {
 	CallGraph call.Graph              // discovered call graph
 	Queries   map[ssa.Value][]Pointer // points-to sets for queried ssa.Values
+	Warnings  []Warning               // warnings of unsoundness
 }
 
 // A Pointer is an equivalence class of pointerlike values.

pointer/gen.go

@@ -87,7 +87,7 @@ func (a *analysis) setValueNode(v ssa.Value, id nodeid, cgn *cgnode) {
 			a.genLoad(cgn, tmp, v, 0, a.sizeof(v.Type()))
 			id = tmp
 		}
-		a.queries[v] = append(a.queries[v], ptr{a, cgn, id})
+		a.result.Queries[v] = append(a.result.Queries[v], ptr{a, cgn, id})
 	}
 }
 

pointer/pointer_test.go

@@ -285,7 +285,6 @@ func doOneInput(input, filename string) bool {
 	}
 
 	var probes []probe
-	var warnings []string
 	var log bytes.Buffer
 
 	// Run the analysis.
@@ -297,11 +296,6 @@ func doOneInput(input, filename string) bool {
 		Print: func(site *ssa.CallCommon, p pointer.Pointer) {
 			probes = append(probes, probe{site, p})
 		},
-		Warn: func(pos token.Pos, format string, args ...interface{}) {
-			msg := fmt.Sprintf(format, args...)
-			fmt.Printf("%s: warning: %s\n", prog.Fset.Position(pos), msg)
-			warnings = append(warnings, msg)
-		},
 	}
 	result := pointer.Analyze(config)
 
@@ -346,7 +340,7 @@ func doOneInput(input, filename string) bool {
 			}
 
 		case "warning":
-			if !checkWarningExpectation(prog, e, warnings) {
+			if !checkWarningExpectation(prog, e, result.Warnings) {
 				ok = false
 			}
 		}
@@ -493,7 +487,7 @@ func checkCallsExpectation(prog *ssa.Program, e *expectation, callgraph call.Gra
 	return false
 }
 
-func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []string) bool {
+func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []pointer.Warning) bool {
 	// TODO(adonovan): check the position part of the warning too?
 	re, err := regexp.Compile(e.args[0])
 	if err != nil {
@@ -506,15 +500,15 @@ func checkWarningExpectation(prog *ssa.Program, e *expectation, warnings []strin
 		return false
 	}
 
-	for _, warning := range warnings {
-		if re.MatchString(warning) {
+	for _, w := range warnings {
+		if re.MatchString(w.Message) {
 			return true
 		}
 	}
 
 	e.errorf("@warning %s expectation not satised; found these warnings though:", strconv.Quote(e.args[0]))
-	for _, warning := range warnings {
-		fmt.Println("\t", warning)
+	for _, w := range warnings {
+		fmt.Printf("%s: warning: %s\n", prog.Fset.Position(w.Pos), w.Message)
 	}
 	return false
 }