code review 12147043: go.tools/ssa: extend debug information to arbitrary ast...

go.tools/ssa: extend debug information to arbitrary ast.Exprs.

CanonicalPos was inadequate since many pairs of instruction share the same pos (e.g. Allocs and Phis).  Instead, we generalize the DebugRef instruction to associate not just Idents but Exprs with ssa.Values.  

We no longer store any DebugRefs for constant expressions, to save space.  (The type and value of such expressions can be obtained by other means, at a cost in complexity.)

Function.ValueForExpr queries the DebugRef info to return the ssa.Value of a given Expr.

Added tests.

Also:
- the DebugInfo flag is now per package, not global.  
   It must be set between Create and Build phases if desired.
- {Value,Instruction}.Pos() documentation updated: we still maintain
  this information in the instruction stream even in non-debug mode,
  but we make fewer claims about its invariants.
- Go and Defer instructions can now use their respective go/defer
   token positions (not the call's lparen), so they do.
- SelectState:
     Posn token.Pos indicates the <- position
     DebugNode ast.Expr is the send stmt or receive expr.
- In building SelectStmt, we introduce extra temporaries in debug
   mode to hold the result of the receive in 'case <-ch' even though
   this value isn't ordinarily needed.
- Use *SelectState (indirectly) since the struct is getting bigger.
- Document some missing instructions in doc.go.

[adonovan, 2013-07-31 16:31:22]

Hello gri@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.tools

[gri, 2013-07-31 16:42:05]

LGTM

https://codereview.appspot.com/12147043/diff/8001/ssa/source.go
File ssa/source.go (right):

https://codereview.appspot.com/12147043/diff/8001/ssa/source.go#newcode140
ssa/source.go:140: e = unparen(e)
do this inside the if?

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go
File ssa/source_test.go (right):

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go#newcode192
ssa/source_test.go:192: t.Errorf("parse error: %s", err)
just t.Error(err)

then you get the position information first - it's clear that it's a parse error

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go#newcode240
ssa/source_test.go:240: } else {
no else

you have a continue before

[adonovan, 2013-07-31 17:10:47]

https://codereview.appspot.com/12147043/diff/8001/ssa/source.go
File ssa/source.go (right):

https://codereview.appspot.com/12147043/diff/8001/ssa/source.go#newcode140
ssa/source.go:140: e = unparen(e)
On 2013/07/31 16:42:06, gri wrote:
> do this inside the if?

Done.

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go
File ssa/source_test.go (right):

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go#newcode192
ssa/source_test.go:192: t.Errorf("parse error: %s", err)
On 2013/07/31 16:42:06, gri wrote:
> just t.Error(err)
> 
> then you get the position information first - it's clear that it's a parse
error

Done, here and elsewhere.

https://codereview.appspot.com/12147043/diff/8001/ssa/source_test.go#newcode240
ssa/source_test.go:240: } else {
On 2013/07/31 16:42:06, gri wrote:
> no else
> 
> you have a continue before

Yes, but target is local to the if statement.

[adonovan, 2013-07-31 17:13:07]

*** Submitted as
https://code.google.com/p/go/source/detail?r=ad296b71409a&repo=tools ***

go.tools/ssa: extend debug information to arbitrary ast.Exprs.

CanonicalPos was inadequate since many pairs of instruction share the same pos
(e.g. Allocs and Phis).  Instead, we generalize the DebugRef instruction to
associate not just Idents but Exprs with ssa.Values.  

We no longer store any DebugRefs for constant expressions, to save space.  (The
type and value of such expressions can be obtained by other means, at a cost in
complexity.)

Function.ValueForExpr queries the DebugRef info to return the ssa.Value of a
given Expr.

Added tests.

Also:
- the DebugInfo flag is now per package, not global.  
   It must be set between Create and Build phases if desired.
- {Value,Instruction}.Pos() documentation updated: we still maintain
  this information in the instruction stream even in non-debug mode,
  but we make fewer claims about its invariants.
- Go and Defer instructions can now use their respective go/defer
   token positions (not the call's lparen), so they do.
- SelectState:
     Posn token.Pos indicates the <- position
     DebugNode ast.Expr is the send stmt or receive expr.
- In building SelectStmt, we introduce extra temporaries in debug
   mode to hold the result of the receive in 'case <-ch' even though
   this value isn't ordinarily needed.
- Use *SelectState (indirectly) since the struct is getting bigger.
- Document some missing instructions in doc.go.

R=gri
CC=golang-dev
https://codereview.appspot.com/12147043